From: James Morris on
On Thu, 12 Nov 2009, Casey Schaufler wrote:

> 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.

Do you see potential for a buffer overrun in this case?

The strings being compared are "sysfs" and the name field of 'struct
file_system_type'. The kernel code elsewhere assumes the latter string to
be a valid zero-terminated string, and we should, too.


- James
--
James Morris
<jmorris(a)namei.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Julia Lawall on
On Fri, 13 Nov 2009, James Morris wrote:

> On Thu, 12 Nov 2009, Casey Schaufler wrote:
>
> > 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.
>
> Do you see potential for a buffer overrun in this case?
>
> The strings being compared are "sysfs" and the name field of 'struct
> file_system_type'. The kernel code elsewhere assumes the latter string to
> be a valid zero-terminated string, and we should, too.

The sizeof only helps for the zero-termination of y, ie "sysfs". Is it
possible for the 0 at the end of an explicit constant string to get
overwritten? If it were the strncmp would be helpful, because the number
of characters to consider would be determined at compile time. If there
is some problem with the name field, the strncmp will look at least to the
end of "sysfs", so the strncmp won't help to keep the character accesses
within the valid characters of name.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Wagner on
Julia Lawall wrote:
> Is it possible for the 0 at the end of an explicit constant string
> to get overwritten?

I don't see any way it could in this case. If there were some other
bug that allowed overwriting the explicit constant string, we'd want
to fix that other bug, but I don't see anything like that in this case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Casey Schaufler on
James Morris wrote:
> On Thu, 12 Nov 2009, Casey Schaufler wrote:
>
>
>> 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.
>>
>
> Do you see potential for a buffer overrun in this case?
>

No, but I hate arguing with people who think that every time
they see strcmp that they have found a security flaw. The
existing code does exactly what it is intended to. Why make
a change that just clutters things up?

> The strings being compared are "sysfs" and the name field of 'struct
> file_system_type'. The kernel code elsewhere assumes the latter string to
> be a valid zero-terminated string, and we should, too.
>
>
> - James
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Wagner on
Casey Schaufler wrote:
>James Morris wrote:
>> Do you see potential for a buffer overrun in this case?
>
>No, but I hate arguing with people who think that every time
>they see strcmp that they have found a security flaw.

So don't argue with those people, then. Those people are
probably deluded or ill-informed, if that's what they think every
time they see strcmp().

If you feel you absolutely must respond to them, send them here and
let them make the case for their position directly, with a concrete
technical argument -- if they have one (which I doubt). Or, better yet,
ignore those people. If they have a kneejerk reaction that "strcmp()
= security flaw", what makes you think they have anything useful to
contribute anyway?

I don't think this concern should have any weight whatsoever in the
decision on whether to patch the code.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/