Messages in this thread | | | From | Andy Lutomirski <> | Date | Wed, 29 Oct 2014 15:24:38 -0700 | Subject | Re: [GIT PULL] Fix for Integrity subsystem null pointer deref |
| |
On Wed, Oct 29, 2014 at 3:23 PM, Dmitry Kasatkin <dmitry.kasatkin@gmail.com> wrote: > On 29 October 2014 23:22, Andy Lutomirski <luto@amacapital.net> wrote: >> On Oct 29, 2014 1:20 PM, "Mimi Zohar" <zohar@linux.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@oracle.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. >
My point is that there's no possible way to tell that only the first byte is read just by reading the function.
--Andy
> - Dmitry > >> >>> >>> Mimi >>> >> -- >> 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 > > > > -- > Thanks, > Dmitry
-- Andy Lutomirski AMA Capital Management, LLC
| |