linux-kernel - Re: [PATCH v2 12/12] use less confusing names for iov

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: <CAHk-=wi_iDAugqFZxTiscsRCNbtARMFiugWtBKO=NqgM-vCVAQ@mail.gmail.com>
Date:   Fri, 28 Oct 2022 11:35:06 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Christoph Hellwig <hch@...radead.org>,
        David Howells <dhowells@...hat.com>, willy@...radead.org,
        dchinner@...hat.com, Steve French <smfrench@...il.com>,
        Shyam Prasad N <nspmangalore@...il.com>,
        Rohith Surabattula <rohiths.msft@...il.com>,
        Jeff Layton <jlayton@...nel.org>,
        Ira Weiny <ira.weiny@...el.com>, linux-cifs@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 12/12] use less confusing names for iov_iter direction initializers

On Fri, Oct 28, 2022 at 10:15 AM Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > I can see the logic: "the destination is the iter, so the source is
> > the bvec".
>
> ???
>
> Wait a sec; bvec is destination - we are going to store data into the page
> hanging off that bvec.

Yeah, no, I'm confused and used confusing language. The bvec is the
only "source" in the sense that it's the original destination.  They
are both the destination for the data itself.

> Umm...  How are you going to e.g. copy from ITER_DISCARD?  I've no problem
> with WARN_ON_ONCE(), but when the operation really can't be done, what
> can we do except returning an error?

Fair enough. But it's the "people got the direction wrong, but the
code worked" case that I would want tyo make sure still works - just
with a warning.

Clearly the ITER_DISCARD didn't work before either, but all the cases
in patches 1-10 were things that _worked_, just with entirely the
wrong ->data_source (aka iov_iter_rw()) value.

So things like copy_to_iter() should warn if it's not a READ (or
ITER_DEST), but it should still copy into the destination described by
the iter, in order to keep broken code working.

That's simply because I worry that your patches 1-10 didn't actually
catch every single case. I'm not actually sure how you found them all
- did you have some automation, or was it with "boot and find warnings
from the first version of patch 11/12"?


> No.  If nothing else, you'll get to split struct msghdr (msg->msg_iter
> different for sendmsg and recvmsg that way) *and* you get to split
> every helper in net/* that doesn't give a damn about the distinction
> (as in "doesn't even look at ->msg_iter", for example).

Gah. Ok. So it's more than just direct_io. Annoying.

              Linus