Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] module: fix validate_section_offset() overflow bug on
 64-bit
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     jeyu@kernel.org, linux-kernel@vger.kernel.org,
        Shuah Khan <skhan@linuxfoundation.org>
References: <20211015205741.38283-1-skhan@linuxfoundation.org>
 <20211015205741.38283-2-skhan@linuxfoundation.org>
 <YWnvN7jHdWHl/05D@bombadil.infradead.org>
From:   Shuah Khan <skhan@linuxfoundation.org>
Message-ID: <42f46af4-4698-f235-9d1a-749774b5cee8@linuxfoundation.org>
Date:   Mon, 18 Oct 2021 11:36:13 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <YWnvN7jHdWHl/05D@bombadil.infradead.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/15/21 3:14 PM, Luis Chamberlain wrote:
> On Fri, Oct 15, 2021 at 02:57:41PM -0600, Shuah Khan wrote:
>> validate_section_offset() uses unsigned long local variable to
>> add/store shdr->sh_offset and shdr->sh_size on all platforms.
>> unsigned long is too short when sh_offset is Elf64_Off which
>> would be the case on 64bit ELF headers.
>>
>> Fix the overflow problem using the right size local variable when
>> CONFIG_64BIT is defined.
>>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> Thanks for doing this! I put this through the 0-day grinder.
> 
> In the meantime, since this talks about a fix, can the commit log be a
> bit more descriptive about the impact of not applying the fix? In what
> situation would not having this patch applied create an issue? Is this
> theoretical or can an issue really happen. Has an issue gone
> undiscovered for a while, and if so what could the consequences
> have been all along?
> 

I found this when I was adding an error message to print the offset and
size.

> And it would seem this issue was found through code inspection, not
> through a real bug, correct? If this can be clarified on the commit log
> as well that would be great!
> 

Sent v2 with updated commit log.

thanks,
-- Shuah