linux-kernel - Re: [PATCH] io_uring/rsrc: send exact nr

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <a263d544-f153-4918-acea-5ce9db6d0d60@kernel.dk>
Date: Wed, 16 Apr 2025 14:30:51 -0600
From: Jens Axboe <axboe@...nel.dk>
To: Pavel Begunkov <asml.silence@...il.com>,
 Nitesh Shetty <nitheshshetty@...il.com>
Cc: Nitesh Shetty <nj.shetty@...sung.com>, gost.dev@...sung.com,
 io-uring@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] io_uring/rsrc: send exact nr_segs for fixed buffer

On 4/16/25 2:29 PM, Pavel Begunkov wrote:
> On 4/16/25 21:01, Jens Axboe wrote:
>> On 4/16/25 1:57 PM, Nitesh Shetty wrote:
> ...
>>>>                  /*
>>>> @@ -1073,7 +1075,6 @@ static int io_import_fixed(int ddir, struct iov_iter *iter,
>>>>                   * since we can just skip the first segment, which may not
>>>>                   * be folio_size aligned.
>>>>                   */
>>>> -               const struct bio_vec *bvec = imu->bvec;
>>>>
>>>>                  /*
>>>>                   * Kernel buffer bvecs, on the other hand, don't necessarily
>>>> @@ -1099,6 +1100,27 @@ static int io_import_fixed(int ddir, struct iov_iter *iter,
>>>>                  }
>>>>          }
>>>>
>>>> +       /*
>>>> +        * Offset trimmed front segments too, if any, now trim the tail.
>>>> +        * For is_kbuf we'll iterate them as they may be different sizes,
>>>> +        * otherwise we can just do straight up math.
>>>> +        */
>>>> +       if (len + offset < imu->len) {
>>>> +               bvec = iter->bvec;
>>>> +               if (imu->is_kbuf) {
>>>> +                       while (len > bvec->bv_len) {
>>>> +                               len -= bvec->bv_len;
>>>> +                               bvec++;
>>>> +                       }
>>>> +                       iter->nr_segs = bvec - iter->bvec;
>>>> +               } else {
>>>> +                       size_t vec_len;
>>>> +
>>>> +                       vec_len = bvec->bv_offset + iter->iov_offset +
>>>> +                                       iter->count + ((1UL << folio_shift) - 1);
>>>> +                       iter->nr_segs = vec_len >> folio_shift;
>>>> +               }
>>>> +       }
>>>>          return 0;
>>>>   }
>>> This might not be needed for is_kbuf , as they already update nr_seg
>>> inside iov_iter_advance.
>>
>> How so? If 'offset' is true, then yes it'd skip the front, but it
>> doesn't skip the end part. And if 'offset' is 0, then no advancing is
>> done in the first place - which does make sense, as it's just advancing
>> from the front.
>>
>>> How about changing something like this ?
>>
>> You can't hide this in the if (offset) section...
> 
> Should we just make it saner first? Sth like these 3 completely
> untested commits
> 
> https://github.com/isilence/linux/commits/rsrc-import-cleanup/
> 
> And then it'll become
> 
> nr_segs = ALIGN(offset + len, 1UL << folio_shift);

Let's please do that, certainly an improvement. Care to send this out? I
can toss them at the testing. And we'd still need that last patch to
ensure the segment count is correct. Honestly somewhat surprised that
the only odd fallout of that is (needlessly) hitting the bio split path.

-- 
Jens Axboe