linux-kernel - Re: [RFC net-next v3 23/29] io_uring: allow to pass addr into sendzc

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: <4eb0adae-660a-3582-df27-d6c254b97adb@samba.org>
Date:   Sat, 13 Aug 2022 10:45:24 +0200
From:   Stefan Metzmacher <metze@...ba.org>
To:     Pavel Begunkov <asml.silence@...il.com>, io-uring@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Jens Axboe <axboe@...nel.dk>, kernel-team@...com
Subject: Re: [RFC net-next v3 23/29] io_uring: allow to pass addr into sendzc

Hi Pavel,

>> Given that this fills in msg almost completely can we also have
>> a version of SENDMSGZC, it would be very useful to also allow
>> msg_control to be passed and as well as an iovec.
>>
>> Would that be possible?
> 
> Right, I left it to follow ups as the series is already too long.
> 
> fwiw, I'm going to also add addr to IORING_OP_SEND.

Given the minimal differences, which were left between
IORING_OP_SENDZC and IORING_OP_SEND, wouldn't it be better
to merge things to IORING_OP_SEND using a IORING_RECVSEND_ZC_NOTIF
as indication to use the notif slot.

It would means we don't need to waste two opcodes for
IORING_OP_SENDZC and IORING_OP_SENDMSGZC (and maybe more)

I also noticed a problem in io_notif_update()

         for (; idx < idx_end; idx++) {
                 struct io_notif_slot *slot = &ctx->notif_slots[idx];

                 if (!slot->notif)
                         continue;
                 if (up->arg)
                         slot->tag = up->arg;
                 io_notif_slot_flush_submit(slot, issue_flags);
         }

  slot->tag = up->arg is skipped if there is no notif already.

So you can't just use a 2 linked sqe's with

IORING_RSRC_UPDATE_NOTIF followed by IORING_OP_SENDZC(with IORING_RECVSEND_NOTIF_FLUSH)

I think the if (!slot->notif) should be moved down a bit.

It would somehow be nice to avoid the notif slots at all and somehow
use some kind of multishot request in order to generate two qces.

I'm also wondering what will happen if a notif will be referenced by the net layer
but the io_uring instance is already closed, wouldn't
io_uring_tx_zerocopy_callback() or __io_notif_complete_tw() crash
because notif->ctx is a stale pointer, of notif itself is already gone...

What do you think?

metze