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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 30 Oct 2014 00:23:55 +0200
From:	Dmitry Kasatkin <dmitry.kasatkin@...il.com>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	LSM List <linux-security-module@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	James Morris <jmorris@...ei.org>,
	"security@...nel.org" <security@...nel.org>
Subject: Re: [GIT PULL] Fix for Integrity subsystem null pointer deref

On 29 October 2014 23:22, Andy Lutomirski <luto@...capital.net> wrote:
> On Oct 29, 2014 1:20 PM, "Mimi Zohar" <zohar@...ux.vnet.ibm.com> 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@...cle.com> 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@...r.kernel.org
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/