Re: [PATCH 04/16] elfops: add strict bounds checking to get/load_section()

[Alan Jenkins <sourcejedi.lkml@xxxxxxxxxxxxxx>]

On 5/18/09, Alan Jenkins <sourcejedi.lkml@xxxxxxxxxxxxxx> wrote:
> On 5/18/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
>> On Mon, 2009-05-18 at 17:14 +0100, Alan Jenkins wrote:
>>> On 5/18/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
>>> > get_section() and load_section() now return NULL if a section header
>>> > is corrupt and points to a block that lies partially or entirely
>>> > outside the file data buffer.
>>>
>>> Great!
>>>
>>> Just out of interest, do you have any ideas about adding bounds
>>> checking for strings?  If I remember correctly, there's still a
>>> possibility for crashes if next_string() is used on a section which is
>>> missing a final NUL terminator.
>>
>> Hmm, you're right ...
>>
>> Would it be terribly ugly to have next_string() check that a terminator
>> exists at the end of the section and if it doesn't, insert one and then
>> write a warning message to the log?
>
> Yeah, terribly ugly :-).

>  That would require mapping the file as copy
> on write (private).  But I _think_ we should be able to run on NOMMU,
> where that is not supported.

Disregard, I thought wrong.  modprobe already modifies module data to
implement the more nefarious options.  Still, I would suggest using
fatal() instead.  We should already have the module filename; we can
output a useful error message and exit(), without returning to the
caller.

> I would prefer that the program refuse to handle the module if it is
> discovered to be corrupt.  I don't think we should try and work around
> such corruption.
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html