On Wed, Oct 29, 2014 at 3:23 PM, Dmitry Kasatkin <[email protected]> wrote: > On 29 October 2014 23:22, Andy Lutomirski <[email protected]> wrote: >> On Oct 29, 2014 1:20 PM, "Mimi Zohar" <[email protected]> wrote: >>> >>> On Wed, 2014-10-29 at 11:51 -0700, Andy Lutomirski wrote: >>> > On Wed, Oct 29, 2014 at 11:36 AM, Dan Carpenter >>> > <[email protected]> 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 [email protected] >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Thanks, > Dmitry -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
