linux-hardening - Re: [PATCH v13 5/9] drm/i915: Check for integer truncation on scatterlist creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Oct 2022 19:38:54 +0300
From:   Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
To:     Jani Nikula <jani.nikula@...el.com>,
        <intel-gfx@...ts.freedesktop.org>
CC:     <linux-kernel@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <mchehab@...nel.org>, <chris@...is-wilson.co.uk>,
        <matthew.auld@...el.com>, <thomas.hellstrom@...ux.intel.com>,
        <nirmoy.das@...el.com>, <airlied@...hat.com>, <daniel@...ll.ch>,
        <andi.shyti@...ux.intel.com>, <andrzej.hajda@...el.com>,
        <keescook@...omium.org>, <mauro.chehab@...ux.intel.com>,
        <linux@...musvillemoes.dk>, <vitor@...saru.org>,
        <dlatypov@...gle.com>, <ndesaulniers@...gle.com>,
        <trix@...hat.com>, <llvm@...ts.linux.dev>,
        <linux-hardening@...r.kernel.org>, <linux-sparse@...r.kernel.org>,
        <nathan@...nel.org>, <gustavoars@...nel.org>,
        <luc.vanoostenryck@...il.com>
Subject: Re: [PATCH v13 5/9] drm/i915: Check for integer truncation on
 scatterlist creation



On 9/28/22 11:51 AM, Jani Nikula wrote:
> On Wed, 28 Sep 2022, Gwan-gyeong Mun <gwan-gyeong.mun@...el.com> wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h
>> index 9ddb3e743a3e..1d1802beb42b 100644
>> --- a/drivers/gpu/drm/i915/i915_scatterlist.h
>> +++ b/drivers/gpu/drm/i915/i915_scatterlist.h
>> @@ -220,4 +220,15 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res,
>>   						     u64 region_start,
>>   						     u32 page_alignment);
>>   
>> +/* Wrap scatterlist.h to sanity check for integer truncation */
>> +typedef unsigned int __sg_size_t; /* see linux/scatterlist.h */
>> +#define sg_alloc_table(sgt, nents, gfp) \
>> +	overflows_type(nents, __sg_size_t) ? -E2BIG \
>> +		: ((sg_alloc_table)(sgt, (__sg_size_t)(nents), gfp))
>> +
>> +#define sg_alloc_table_from_pages_segment(sgt, pages, npages, offset, size, max_segment, gfp) \
>> +	overflows_type(npages, __sg_size_t) ? -E2BIG \
>> +		: ((sg_alloc_table_from_pages_segment)(sgt, pages, (__sg_size_t)(npages), offset, \
>> +						       size, max_segment, gfp))
>> +
>>   #endif
> 
> No. I don't think we should shadow sg_alloc_table() and
> sg_alloc_table_from_pages_segment().
> 
> Either get this in scatterlist.h (preferred) or prefix with i915_ or
> whatever to indicate it's our local thing.
> 
> i915_scatterlist.h already has too much scatterlist "namespace" abuse
> that I'd rather see gone than violated more.
> 
> 
Hi Jani,
Thanks for you comments.

I will update this patch by removing the shadowing of 
sg_alloc_table_from_pages_segment() / sg_alloc_table(), so that the 
caller checks when overflow checking is required.

Br,
G.G.

> BR,
> Jani.
> 
> 
>