lkml.org 
[lkml]   [2009]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] security/selinux: decrement sizeof size in strncmp
    Julia Lawall wrote:
    > On Thu, 12 Nov 2009, Serge E. Hallyn wrote:
    >
    >
    >> Quoting James Morris (jmorris@namei.org):
    >>
    >>> On Thu, 12 Nov 2009, Julia Lawall wrote:
    >>>
    >>>
    >>>> From: Julia Lawall <julia@diku.dk>
    >>>>
    >>>> 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@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    >
    >
    >



    \
     
     \ /
      Last update: 2009-11-12 17:25    [W:0.163 / U:1.312 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site