[PATCH] ifplugd
Denys Vlasenko
vda.linux at googlemail.com
Fri Apr 9 21:27:25 UTC 2010
On Wednesday 07 April 2010 20:02, Maxim Kryžanovský wrote:
> > > > Is this safe?
> > > >
> > > > if (strncmp(G.iface, RTA_DATA(attr), len) == 0)
> > > >
> > > > What if RTA_DATA(attr) = "if", len = 2, and G.iface = "if0"?
> > > > Or does kernel pass attr with NUL included?
> >
> > > Yes, it is not safe. It needs to compare their lengths:
> > > int iface_len = strlen(G.iface);
> > > if (iface_len == len && strncmp(G.iface, RTA_DATA(attr), len) == 0)
> >
> > Ok, now what if RTA_DATA(attr) = "if\0", len = 3, and G.iface = "if"?
> > Your code will think that strings aren't equal. Maybe
> >
> > if (iface_len <= len && strncmp(G.iface, RTA_DATA(attr), len) == 0)
>
> Hi Denis.
>
> The kernel pass attr with NUL included, but I could not determine
> whether does this always. If so, the NUL can be omitted from the total
> length to avoid the need to compare strings with different lengths.
>
> diff --git a/networking/ifplugd.c b/networking/ifplugd.c
> index 41b04c4..dcb6d4d 100644
> --- a/networking/ifplugd.c
> +++ b/networking/ifplugd.c
> @@ -525,10 +525,10 @@ static NOINLINE int check_existence_through_netlink(void)
>
> while (RTA_OK(attr, attr_len)) {
> if (attr->rta_type == IFLA_IFNAME) {
> - int len = RTA_PAYLOAD(attr);
> + int len = RTA_PAYLOAD(attr) - 1;
> if (len > IFNAMSIZ)
> len = IFNAMSIZ;
> - if (iface_len <= len
> + if (iface_len == len
> && strncmp(G.iface, RTA_DATA(attr), len) == 0
> ) {
> G.iface_exists = (mhdr->nlmsg_type == RTM_NEWLINK);
This does not look right. Now if kernel reports "if1", len=3,
you will consider it == "if".
Current code treats "if1",len=3 and "if1\0",len=4 as the same string,
"if1", which seems to be intuitively correct for C conventions.
I just don't see the case you are trying to fix. Describe it,
give the values where it works wrongly:
G.iface = "??", RTA_DATA(attr) = "??", len = ??
Regrding:
+#if ENABLE_FEATURE_PIDFILE
write_pidfile(pidfile_name);
+#endif
This is not needed since libbb.h has
#define write_pidfile(path) ((void)0)
#define remove_pidfile(path) ((void)0)
if ENABLE_FEATURE_PIDFILE is 0.
--
vda
More information about the busybox
mailing list