Re: [GIT PULL] Fix for Integrity subsystem null pointer deref

From: Dmitry Kasatkin
Date: Wed Oct 29 2014 - 18:24:01 EST


On 29 October 2014 23:22, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
> On Oct 29, 2014 1:20 PM, "Mimi Zohar" <zohar@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> On Wed, 2014-10-29 at 11:51 -0700, Andy Lutomirski wrote:
>> > On Wed, Oct 29, 2014 at 11:36 AM, Dan Carpenter
>> > <dan.carpenter@xxxxxxxxxx> wrote:
>> > > On Wed, Oct 29, 2014 at 09:23:45AM -0700, Andy Lutomirski wrote:
>> > >> I have no idea what the semantics are. All I'm saying is that it
>> > >> looks like the code still accesses memory past the end of the buffer.
>> > >> The buffer isn't a null pointer, so the symptom is different, but it
>> > >> may still be a security bug.
>> > >>
>> > >> --Andy
>> > >
>> > > It only reads one byte into the struct "xattr_data->type" so checking
>> > > for non-zero is sufficient and the patch is fine.
>> >
>> > Indeed. Still... eww. I don't like code that, upon local inspection,
>> > is apparently wrong, even though it's coincidentally correct due to
>> > some other far away condition.
>>
>> No, the code may be incomplete, but definitely not wrong.
>
> I said "apparently wrong" instead of "wrong" for a reason :)

I see there is a long discussion about this patch.

Actually using something like this "if (xattr_value_len <
sizeof(struct evm_ima_xattr_data))" or using sizeof (struct
signature_v2_hdr)
in this function does not give too much.
xattr value is variable length value and having that statement false
absolutely does not mean that the value is correct.
It can be even a random garbage of the correct size.
This particular function checks the first byte only so the test is good enough.

- Dmitry

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



--
Thanks,
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/