From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD5E1C00140 for ; Tue, 2 Aug 2022 23:25:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230362AbiHBXZW (ORCPT ); Tue, 2 Aug 2022 19:25:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35774 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229648AbiHBXZW (ORCPT ); Tue, 2 Aug 2022 19:25:22 -0400 Received: from gnuweeb.org (gnuweeb.org [51.81.211.47]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 859665F40; Tue, 2 Aug 2022 16:25:20 -0700 (PDT) Received: from [192.168.88.254] (unknown [125.160.110.15]) by gnuweeb.org (Postfix) with ESMTPSA id EF8CE8060F; Tue, 2 Aug 2022 23:25:16 +0000 (UTC) X-GW-Data: lPqxHiMPbJw1wb7CM9QUryAGzr0yq5atzVDdxTR0iA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1659482719; bh=+au6dQlBqQIHJQgyGdW4Nb34wf6bajMRt7oudmw/yo4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=gFXTqEcTqyl0/ej7COyaD6isAwgmie7aA2fAMxbf4A3wZbyjrmHLRchIVlj/RsV/p OMXY0fANBx40iqGa2gX/Cxvx3H864UliDnl7DECdpDlkON2s/83iPgnLvxWKHND1wT BotuQS5+le1vHJzh2whp1N2fVFBPHbDMwB/iJulNHL2dSwuJLzXIA44Y1nGxr7v/sf nnKm63FpaQEQQZudRwEsDNoDrzZfGRAE8xU+xvfRAPqdpO4BCt2CHRRuLUoLHmyLkM DpWO0SFIRp9B615YZ6mNwLgE1oqW0Rm6WocN8i+49AmmIfMAylZLBBhDyHLpewIm8c BPf/hx2vhUoiw== Message-ID: <05dfe735-d146-ad5c-2f98-940032fd1f48@gnuweeb.org> Date: Wed, 3 Aug 2022 06:25:14 +0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCHv2 6/7] io_uring: add support for dma pre-mapping Content-Language: en-US To: Keith Busch Cc: Facebook Kernel Team , Jens Axboe , Christoph Hellwig , Al Viro , Keith Busch , Fernanda Ma'rouf , io-uring Mailing List , Linux Block Mailing List , Linux fsdevel Mailing List , Linux NVME Mailing List References: <20220802193633.289796-1-kbusch@fb.com> <20220802193633.289796-7-kbusch@fb.com> From: Ammar Faizi In-Reply-To: <20220802193633.289796-7-kbusch@fb.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: io-uring@vger.kernel.org On 8/3/22 2:36 AM, Keith Busch wrote: > From: Keith Busch > > Provide a new register operation that can request to pre-map a known > bvec to the requested fixed file's specific implementation. If > successful, io_uring will use the returned dma tag for future fixed > buffer requests to the same file. > > Signed-off-by: Keith Busch [...] > +static int io_register_map_buffers(struct io_ring_ctx *ctx, void __user *arg) > +{ > + struct io_uring_map_buffers map; > + struct io_fixed_file *file_slot; > + struct file *file; > + int ret, i; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + ret = get_map_range(ctx, &map, arg); > + if (ret < 0) > + return ret; > + > + file_slot = io_fixed_file_slot(&ctx->file_table, > + array_index_nospec(map.fd, ctx->nr_user_files)); > + if (!file_slot || !file_slot->file_ptr) > + return -EBADF; The @file_slot NULL-check doesn't make sense. The definition of io_fixed_file_slot() is: static inline struct io_fixed_file * io_fixed_file_slot(struct io_file_table *table, unsigned i) { return &table->files[i]; } which takes the address of an element in the array. So @file_slot should never be NULL, if it ever be, something has gone wrong. If you ever had @ctx->file_table.files being NULL in this path, you should NULL-check the @->files itself, *not* the return value of io_fixed_file_slot(). IOW: ... // NULL check here. if (!ctx->file_table.files) return -EBADF; file_slot = io_fixed_file_slot(&ctx->file_table, array_index_nospec(map.fd, ctx->nr_user_files)); if (!file_slot->file_ptr) return -EBADF; ... > for (i = 0; i < ctx->nr_user_files; i++) { > - struct file *file = io_file_from_index(&ctx->file_table, i); > + struct io_fixed_file *f = io_fixed_file_slot(&ctx->file_table, i); > + struct file *file; > > - if (!file) > + if (!f) > continue; The same thing, this @f NULL-check is not needed. > - if (io_fixed_file_slot(&ctx->file_table, i)->file_ptr & FFS_SCM) > + if (f->file_ptr & FFS_SCM) > continue; > + > + io_dma_unmap_file(ctx, f); > + file = io_file_from_fixed(f); > io_file_bitmap_clear(&ctx->file_table, i); > fput(file); > } -- Ammar Faizi