From patchwork Mon Jan 18 16:30:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 365856 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C9EBC4332D for ; Mon, 18 Jan 2021 16:31:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8603F22C7E for ; Mon, 18 Jan 2021 16:31:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406650AbhARQbT (ORCPT ); Mon, 18 Jan 2021 11:31:19 -0500 Received: from smtp.infotech.no ([82.134.31.41]:45718 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406725AbhARQbH (ORCPT ); Mon, 18 Jan 2021 11:31:07 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 73FE92042B2; Mon, 18 Jan 2021 17:30:21 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id biqWdKW9SgrG; Mon, 18 Jan 2021 17:30:17 +0100 (CET) Received: from xtwo70.bingwo.ca (host-104-157-204-209.dyn.295.ca [104.157.204.209]) by smtp.infotech.no (Postfix) with ESMTPA id 6C149204278; Mon, 18 Jan 2021 17:30:14 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org, linux-block@vger.kernel.org, target-devel@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, bostroesser@gmail.com, ddiss@suse.de, bvanassche@acm.org, jgg@ziepe.ca Subject: [PATCH v6 1/4] sgl_alloc_order: remove 4 GiB limit, sgl_free() warning Date: Mon, 18 Jan 2021 11:30:03 -0500 Message-Id: <20210118163006.61659-2-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210118163006.61659-1-dgilbert@interlog.com> References: <20210118163006.61659-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org This patch fixes a check done by sgl_alloc_order() before it starts any allocations. The comment in the original said: "Check for integer overflow" but the check itself contained an integer overflow! The right hand side (rhs) of the expression in the condition is resolved as u32 so it could not exceed UINT32_MAX (4 GiB) which means 'length' could not exceed that value. If that was the intention then the comment above it could be dropped and the condition rewritten more clearly as: if (length > UINT32_MAX) <>; After several flawed attempts to detect overflow, take the fastest route by stating as a pre-condition that the 'order' function argument cannot exceed 16 (2^16 * 4k = 256 MiB). This function may be used to replace vmalloc(unsigned long) for a large allocation (e.g. a ramdisk). vmalloc has no limit at 4 GiB so it seems unreasonable that: sgl_alloc_order(unsigned long long length, ....) does. sgl_s made with sgl_alloc_order() have equally sized segments placed in a scatter gather array. That allows O(1) navigation around a big sgl using some simple integer arithmetic. Revise some of this function's description to more accurately reflect what this function is doing. An earlier patch fixed a memory leak in sg_alloc_order() due to the misuse of sgl_free(). Take the opportunity to put a one line comment above sgl_free()'s declaration warning that it is not suitable when order > 0 . Reviewed-by: Bodo Stroesser Signed-off-by: Douglas Gilbert --- include/linux/scatterlist.h | 1 + lib/scatterlist.c | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 6f70572b2938..8adff41f7cfa 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -302,6 +302,7 @@ struct scatterlist *sgl_alloc(unsigned long long length, gfp_t gfp, unsigned int *nent_p); void sgl_free_n_order(struct scatterlist *sgl, int nents, int order); void sgl_free_order(struct scatterlist *sgl, int order); +/* Only use sgl_free() when order is 0 */ void sgl_free(struct scatterlist *sgl); #endif /* CONFIG_SGL_ALLOC */ diff --git a/lib/scatterlist.c b/lib/scatterlist.c index a59778946404..24ea2d31a405 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -554,13 +554,16 @@ EXPORT_SYMBOL(sg_alloc_table_from_pages); #ifdef CONFIG_SGL_ALLOC /** - * sgl_alloc_order - allocate a scatterlist and its pages + * sgl_alloc_order - allocate a scatterlist with equally sized elements each + * of which has 2^@order continuous pages * @length: Length in bytes of the scatterlist. Must be at least one - * @order: Second argument for alloc_pages() + * @order: Second argument for alloc_pages(). Each sgl element size will + * be (PAGE_SIZE*2^@order) bytes. @order must not exceed 16. * @chainable: Whether or not to allocate an extra element in the scatterlist - * for scatterlist chaining purposes + * for scatterlist chaining purposes * @gfp: Memory allocation flags - * @nent_p: [out] Number of entries in the scatterlist that have pages + * @nent_p: [out] Number of entries in the scatterlist that have pages. + * Ignored if NULL is given. * * Returns: A pointer to an initialized scatterlist or %NULL upon failure. */ @@ -574,15 +577,11 @@ struct scatterlist *sgl_alloc_order(unsigned long long length, u32 elem_len; nent = round_up(length, PAGE_SIZE << order) >> (PAGE_SHIFT + order); - /* Check for integer overflow */ - if (length > (nent << (PAGE_SHIFT + order))) - return NULL; - nalloc = nent; if (chainable) { - /* Check for integer overflow */ - if (nalloc + 1 < nalloc) + if (check_add_overflow(nent, 1U, &nalloc)) return NULL; - nalloc++; + } else { + nalloc = nent; } sgl = kmalloc_array(nalloc, sizeof(struct scatterlist), gfp & ~GFP_DMA); From patchwork Mon Jan 18 16:30:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 365855 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5FDC6C433DB for ; Mon, 18 Jan 2021 16:33:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1F9BE22BE9 for ; Mon, 18 Jan 2021 16:33:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406777AbhARQbx (ORCPT ); Mon, 18 Jan 2021 11:31:53 -0500 Received: from smtp.infotech.no ([82.134.31.41]:45763 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727050AbhARQbs (ORCPT ); Mon, 18 Jan 2021 11:31:48 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 55D67204295; Mon, 18 Jan 2021 17:30:32 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hNUSQtqmjh3E; Mon, 18 Jan 2021 17:30:28 +0100 (CET) Received: from xtwo70.bingwo.ca (host-104-157-204-209.dyn.295.ca [104.157.204.209]) by smtp.infotech.no (Postfix) with ESMTPA id 666E4204279; Mon, 18 Jan 2021 17:30:25 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org, linux-block@vger.kernel.org, target-devel@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, bostroesser@gmail.com, ddiss@suse.de, bvanassche@acm.org, jgg@ziepe.ca Subject: [PATCH v6 4/4] scatterlist: add sgl_memset() Date: Mon, 18 Jan 2021 11:30:06 -0500 Message-Id: <20210118163006.61659-5-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210118163006.61659-1-dgilbert@interlog.com> References: <20210118163006.61659-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The existing sg_zero_buffer() function is a bit restrictive. For example protection information (PI) blocks are usually initialized to 0xff bytes. As its name suggests sgl_memset() is modelled on memset(). One difference is the type of the val argument which is u8 rather than int. Plus it returns the number of bytes (over)written. Change implementation of sg_zero_buffer() to call this new function. Reviewed-by: Bodo Stroesser Signed-off-by: Douglas Gilbert --- include/linux/scatterlist.h | 20 +++++++++- lib/scatterlist.c | 79 +++++++++++++++++++++---------------- 2 files changed, 62 insertions(+), 37 deletions(-) diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 71be65f9ebb5..69e87280b44d 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -318,8 +318,6 @@ size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents, const void *buf, size_t buflen, off_t skip); size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen, off_t skip); -size_t sg_zero_buffer(struct scatterlist *sgl, unsigned int nents, - size_t buflen, off_t skip); size_t sgl_copy_sgl(struct scatterlist *d_sgl, unsigned int d_nents, off_t d_skip, struct scatterlist *s_sgl, unsigned int s_nents, off_t s_skip, @@ -333,6 +331,24 @@ bool sgl_compare_sgl_idx(struct scatterlist *x_sgl, unsigned int x_nents, off_t struct scatterlist *y_sgl, unsigned int y_nents, off_t y_skip, size_t n_bytes, size_t *miscompare_idx); +size_t sgl_memset(struct scatterlist *sgl, unsigned int nents, off_t skip, + u8 val, size_t n_bytes); + +/** + * sg_zero_buffer - Zero-out a part of a SG list + * @sgl: The SG list + * @nents: Number of SG entries + * @buflen: The number of bytes to zero out + * @skip: Number of bytes to skip before zeroing + * + * Returns the number of bytes zeroed. + **/ +static inline size_t sg_zero_buffer(struct scatterlist *sgl, unsigned int nents, + size_t buflen, off_t skip) +{ + return sgl_memset(sgl, nents, skip, 0, buflen); +} + /* * Maximum number of entries that will be allocated in one piece, if * a list larger than this is required then chaining will be utilized. diff --git a/lib/scatterlist.c b/lib/scatterlist.c index e3182de753d0..7e6acc67e9f6 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -1023,41 +1023,6 @@ size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, } EXPORT_SYMBOL(sg_pcopy_to_buffer); -/** - * sg_zero_buffer - Zero-out a part of a SG list - * @sgl: The SG list - * @nents: Number of SG entries - * @buflen: The number of bytes to zero out - * @skip: Number of bytes to skip before zeroing - * - * Returns the number of bytes zeroed. - **/ -size_t sg_zero_buffer(struct scatterlist *sgl, unsigned int nents, - size_t buflen, off_t skip) -{ - unsigned int offset = 0; - struct sg_mapping_iter miter; - unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG; - - sg_miter_start(&miter, sgl, nents, sg_flags); - - if (!sg_miter_skip(&miter, skip)) - return false; - - while (offset < buflen && sg_miter_next(&miter)) { - unsigned int len; - - len = min(miter.length, buflen - offset); - memset(miter.addr, 0, len); - - offset += len; - } - - sg_miter_stop(&miter); - return offset; -} -EXPORT_SYMBOL(sg_zero_buffer); - /** * sgl_copy_sgl - Copy over a destination sgl from a source sgl * @d_sgl: Destination sgl @@ -1240,3 +1205,47 @@ bool sgl_compare_sgl(struct scatterlist *x_sgl, unsigned int x_nents, off_t x_sk return sgl_compare_sgl_idx(x_sgl, x_nents, x_skip, y_sgl, y_nents, y_skip, n_bytes, NULL); } EXPORT_SYMBOL(sgl_compare_sgl); + +/** + * sgl_memset - set byte 'val' up to n_bytes times on SG list + * @sgl: The SG list + * @nents: Number of SG entries in sgl + * @skip: Number of bytes to skip before starting + * @val: byte value to write to sgl + * @n_bytes: The (maximum) number of bytes to modify + * + * Returns: + * The number of bytes written. + * + * Notes: + * Stops writing if either sgl or n_bytes is exhausted. If n_bytes is + * set SIZE_MAX then val will be written to each byte until the end + * of sgl. + * + * The notes in sgl_copy_sgl() about large sgl_s _applies here as well. + * + **/ +size_t sgl_memset(struct scatterlist *sgl, unsigned int nents, off_t skip, + u8 val, size_t n_bytes) +{ + size_t offset = 0; + size_t len; + struct sg_mapping_iter miter; + + if (n_bytes == 0) + return 0; + sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG); + if (!sg_miter_skip(&miter, skip)) + goto fini; + + while ((offset < n_bytes) && sg_miter_next(&miter)) { + len = min(miter.length, n_bytes - offset); + memset(miter.addr, val, len); + offset += len; + } +fini: + sg_miter_stop(&miter); + return offset; +} +EXPORT_SYMBOL(sgl_memset); +