On 04/29/16 at 10:30am, Russell King - ARM Linux wrote: > > > > > @@ -169,6 +169,8 @@ int sanity_check_segment_list(struct kimage *image) > > > > > > > > > > mstart = image->segment[i].mem; > > > > > mend = mstart + image->segment[i].memsz; > > > > > + if (mstart > mend) > > > > > + return result; > > > > > > > > The type of image->segment[i].memsz is unsigned. So it is no need to > > > > have a test here. > > > > > > Absolutely wrong. Consider the case: > > > > > > segment[i].mem = 0xfff00000; > > > segment[i].size = 0x00200000; > > > > > > Here, mstart will be 0xfff00000, and mend will be 0x00100000. Just > > > because it's some random type does not make things magically work. > > > > Hi, Russell. > > > > Do you mean in PAE mode? If so, we will be in big trouble, because there > > are a lot of functions which use unsigned long to store memory address, > > and this type is 32 bit in PAE mode. > > This is basic input validation stuff, it's got nothing to do with whether > we're in PAE mode. If we get passed such a segment as I illustrate above, > we should detect and fail it, just as we detect and fail other similar > errors. > > I'm not sure what the big deal here is. This is basic validation checks > for stuff coming from userspace which the kernel should be doing as a > matter of course to protect itself. Hi, Russell. Thanks for your reply. I'm fine with this basic test now. Please feel free to add Reviewed-by: Minfei Huang <mhuang at redhat.com>. Thanks Minfei
