From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755257AbdCGJAQ (ORCPT ); Tue, 7 Mar 2017 04:00:16 -0500 Received: from mga06.intel.com ([134.134.136.31]:45023 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754978AbdCGJAK (ORCPT ); Tue, 7 Mar 2017 04:00:10 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,257,1484035200"; d="scan'208";a="56999099" Subject: Re: [Intel-gfx] [PATCH 2/4] lib/scatterlist: Avoid potential scatterlist entry overflow To: Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org References: <1484575930-6810-1-git-send-email-tvrtko.ursulin@linux.intel.com> <1484575930-6810-2-git-send-email-tvrtko.ursulin@linux.intel.com> Cc: linux-kernel@vger.kernel.org, Masahiro Yamada , Andy Shevchenko , Joonas Lahtinen , Chris Wilson From: Tvrtko Ursulin Message-ID: <21a59594-a4bc-4d94-ecc0-ebf87cbec35a@linux.intel.com> Date: Tue, 7 Mar 2017 08:58:36 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <1484575930-6810-2-git-send-email-tvrtko.ursulin@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 16/01/2017 14:12, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Since the scatterlist length field is an unsigned int, make > sure that sg_alloc_table_from_pages does not overflow it while > coallescing pages to a single entry. > > v2: Drop reference to future use. Use UINT_MAX. > v3: max_segment must be page aligned. > v4: Do not rely on compiler to optimise out the rounddown. > (Joonas Lahtinen) > v5: Simplified loops and use post-increments rather than > pre-increments. Use PAGE_MASK and fix comment typo. > (Andy Shevchenko) > Signed-off-by: Tvrtko Ursulin > Cc: Masahiro Yamada > Cc: linux-kernel@vger.kernel.org > Reviewed-by: Chris Wilson (v2) > Cc: Joonas Lahtinen > Cc: Andy Shevchenko Anyone in the mood for reviewing from here to the end of the series? Regards, Tvrtko > --- > include/linux/scatterlist.h | 6 ++++++ > lib/scatterlist.c | 31 ++++++++++++++++++++----------- > 2 files changed, 26 insertions(+), 11 deletions(-) > > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index c981bee1a3ae..4768eeeb7054 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -21,6 +21,12 @@ struct scatterlist { > }; > > /* > + * Since the above length field is an unsigned int, below we define the maximum > + * length in bytes that can be stored in one scatterlist entry. > + */ > +#define SCATTERLIST_MAX_SEGMENT (UINT_MAX & PAGE_MASK) > + > +/* > * These macros should be used after a dma_map_sg call has been done > * to get bus addresses of each of the SG entries and their lengths. > * You should only work with the number of sg entries dma_map_sg > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > index e05e7fc98892..65f375645df5 100644 > --- a/lib/scatterlist.c > +++ b/lib/scatterlist.c > @@ -394,17 +394,22 @@ int sg_alloc_table_from_pages(struct sg_table *sgt, > unsigned int offset, unsigned long size, > gfp_t gfp_mask) > { > - unsigned int chunks; > - unsigned int i; > - unsigned int cur_page; > + const unsigned int max_segment = SCATTERLIST_MAX_SEGMENT; > + unsigned int chunks, cur_page, seg_len, i; > int ret; > struct scatterlist *s; > > /* compute number of contiguous chunks */ > chunks = 1; > - for (i = 1; i < n_pages; ++i) > - if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) > - ++chunks; > + seg_len = 0; > + for (i = 1; i < n_pages; i++) { > + seg_len += PAGE_SIZE; > + if (seg_len >= max_segment || > + page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) { > + chunks++; > + seg_len = 0; > + } > + } > > ret = sg_alloc_table(sgt, chunks, gfp_mask); > if (unlikely(ret)) > @@ -413,17 +418,21 @@ int sg_alloc_table_from_pages(struct sg_table *sgt, > /* merging chunks and putting them into the scatterlist */ > cur_page = 0; > for_each_sg(sgt->sgl, s, sgt->orig_nents, i) { > - unsigned long chunk_size; > - unsigned int j; > + unsigned int j, chunk_size; > > /* look for the end of the current chunk */ > - for (j = cur_page + 1; j < n_pages; ++j) > - if (page_to_pfn(pages[j]) != > + seg_len = 0; > + for (j = cur_page + 1; j < n_pages; j++) { > + seg_len += PAGE_SIZE; > + if (seg_len >= max_segment || > + page_to_pfn(pages[j]) != > page_to_pfn(pages[j - 1]) + 1) > break; > + } > > chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset; > - sg_set_page(s, pages[cur_page], min(size, chunk_size), offset); > + sg_set_page(s, pages[cur_page], > + min_t(unsigned long, size, chunk_size), offset); > size -= chunk_size; > offset = 0; > cur_page = j; >