Julia Lawall wrote: > On Thu, 12 Nov 2009, Serge E. Hallyn wrote: > > >> Quoting James Morris (jmorris@xxxxxxxxx): >> >>> On Thu, 12 Nov 2009, Julia Lawall wrote: >>> >>> >>>> From: Julia Lawall <julia@xxxxxxx> >>>> >>>> As observed by Joe Perches, sizeof of a constant string includes the >>>> trailing 0. If what is wanted is to check the initial characters of >>>> another string, this trailing 0 should not be taken into account. If an >>>> exact match is wanted, strcmp should be used instead. >>>> >>>> --- a/security/selinux/hooks.c >>>> +++ b/security/selinux/hooks.c >>>> @@ -448,7 +448,7 @@ static int sb_finish_set_opts(struct sup >>>> sbsec->flags &= ~SE_SBLABELSUPP; >>>> >>>> /* Special handling for sysfs. Is genfs but also has setxattr handler*/ >>>> - if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0) >>>> + if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs") - 1) == 0) >>>> sbsec->flags |= SE_SBLABELSUPP; >>>> >>> Shouldn't this be a simple strcmp() ? >>> >> Yes I think so. >> >> Julia seems to be arguing that if a module introduces a new fs with >> name 'sysfs_foo' then this check should match that fs too (since >> for sysfs, sb->s_type->name = "sysfs" which also has a trailing \0, >> so for the regular sysfs her patch makes no practical difference). >> > > If strcmp is what is wanted, I can make a patch for that. > I strongly suggest that this is not what is wanted. strcmp(x,y) and strncmp(x,y,sizeof(y)) are functionally equivalent and strcmp has a bad reputation in the security community because it is associated with potential buffer overrun issues. strncmp(x,y,sizeof(y)-1) is not the same and is more cluttered as well. This code does not need to be changed. It does what it is intended to do in a strait-forward manner. > julia > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html