ifstated(8): Fix Hard-Coded Message Size

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

ifstated(8): Fix Hard-Coded Message Size

Vadim Penzin
The following patch replaces the hard-coded message size of 2048 bytes
in ifstated.c:rt_msg_handler() with RTM_MAXSIZE (which is currently
defined as 2048 bytes) that, I believe, was the intent of the author.

Index: ifstated.c
===================================================================
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.64
diff -u -p -r1.64 ifstated.c
--- ifstated.c 28 Jun 2019 13:32:47 -0000 1.64
+++ ifstated.c 14 Jul 2019 14:22:32 -0000
@@ -236,7 +236,7 @@ load_config(void)
  void
  rt_msg_handler(int fd, short event, void *arg)
  {
- char msg[2048];
+ char msg[RTM_MAXSIZE];
  struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
  struct if_msghdr ifm;
  struct if_announcemsghdr ifan;

Reply | Threaded
Open this post in threaded view
|

Re: ifstated(8): Fix Hard-Coded Message Size

Claudio Jeker
On Sun, Jul 14, 2019 at 05:28:17PM +0300, Vadim Penzin wrote:

> The following patch replaces the hard-coded message size of 2048 bytes in
> ifstated.c:rt_msg_handler() with RTM_MAXSIZE (which is currently defined as
> 2048 bytes) that, I believe, was the intent of the author.
>
> Index: ifstated.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 ifstated.c
> --- ifstated.c 28 Jun 2019 13:32:47 -0000 1.64
> +++ ifstated.c 14 Jul 2019 14:22:32 -0000
> @@ -236,7 +236,7 @@ load_config(void)
>  void
>  rt_msg_handler(int fd, short event, void *arg)
>  {
> - char msg[2048];
> + char msg[RTM_MAXSIZE];
>   struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
>   struct if_msghdr ifm;
>   struct if_announcemsghdr ifan;
>

I don't think RTM_MAXSIZE should be used outside of the kernel.
Especially for reading messages. I would leave the code as is.

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: ifstated(8): Fix Hard-Coded Message Size

Vadim Penzin


On 7/15/19 10:15 AM, Claudio Jeker wrote:

> On Sun, Jul 14, 2019 at 05:28:17PM +0300, Vadim Penzin wrote:
>> The following patch replaces the hard-coded message size of 2048 bytes in
>> ifstated.c:rt_msg_handler() with RTM_MAXSIZE (which is currently defined as
>> 2048 bytes) that, I believe, was the intent of the author.
>>
>> Index: ifstated.c
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
>> retrieving revision 1.64
>> diff -u -p -r1.64 ifstated.c
>> --- ifstated.c 28 Jun 2019 13:32:47 -0000 1.64
>> +++ ifstated.c 14 Jul 2019 14:22:32 -0000
>> @@ -236,7 +236,7 @@ load_config(void)
>>   void
>>   rt_msg_handler(int fd, short event, void *arg)
>>   {
>> - char msg[2048];
>> + char msg[RTM_MAXSIZE];
>>   struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
>>   struct if_msghdr ifm;
>>   struct if_announcemsghdr ifan;
>>
>
> I don't think RTM_MAXSIZE should be used outside of the kernel.
> Especially for reading messages. I would leave the code as is.

In that case, please hide RTM_MAXSIZE using _KERNEL.  The definition of
RTM_MAXSIZE is right above RTM_ADD which is user-visible according to
the current documentation.

What about other defines?  Can user-space programs use RTM_VERSION, for
instance?

Reply | Threaded
Open this post in threaded view
|

Re: ifstated(8): Fix Hard-Coded Message Size

Claudio Jeker
On Mon, Jul 15, 2019 at 10:36:28AM +0300, Vadim Penzin wrote:

>
>
> On 7/15/19 10:15 AM, Claudio Jeker wrote:
> > On Sun, Jul 14, 2019 at 05:28:17PM +0300, Vadim Penzin wrote:
> > > The following patch replaces the hard-coded message size of 2048 bytes in
> > > ifstated.c:rt_msg_handler() with RTM_MAXSIZE (which is currently defined as
> > > 2048 bytes) that, I believe, was the intent of the author.
> > >
> > > Index: ifstated.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> > > retrieving revision 1.64
> > > diff -u -p -r1.64 ifstated.c
> > > --- ifstated.c 28 Jun 2019 13:32:47 -0000 1.64
> > > +++ ifstated.c 14 Jul 2019 14:22:32 -0000
> > > @@ -236,7 +236,7 @@ load_config(void)
> > >   void
> > >   rt_msg_handler(int fd, short event, void *arg)
> > >   {
> > > - char msg[2048];
> > > + char msg[RTM_MAXSIZE];
> > >   struct rt_msghdr *rtm = (struct rt_msghdr *)&msg;
> > >   struct if_msghdr ifm;
> > >   struct if_announcemsghdr ifan;
> > >
> >
> > I don't think RTM_MAXSIZE should be used outside of the kernel.
> > Especially for reading messages. I would leave the code as is.
>
> In that case, please hide RTM_MAXSIZE using _KERNEL.  The definition of
> RTM_MAXSIZE is right above RTM_ADD which is user-visible according to the
> current documentation.

I actually consider to move it into rtsock.c since that is the only place
it is used.
 
> What about other defines?  Can user-space programs use RTM_VERSION, for
> instance?

Yes. They should use it so the ABI used is clear.

--
:wq Claudio