linux-kernel - Re: [PATCH 002/437] fs: add generic read/write iterator helpers

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]

Date: Tue, 16 Apr 2024 00:42:57 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Jens Axboe <axboe@...nel.dk>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 002/437] fs: add generic read/write iterator helpers

On Mon, Apr 15, 2024 at 03:16:12PM -0600, Jens Axboe wrote:

> The old read/write path only handled user pointers, with the exception
> being bvecs mapped on the io_uring side (which the io_uring version
> dealt with) which also originally came from user pointers. So just user
> memory. Why would we need to expand that now? Who would be doing
> ->read_iter() or ->write_iter() with something that isn't either UBUF or
> IOVEC? Because that would break horrible as it is in the current kernel.

No, it will not.  And yes, it does happen.  Look, for example, at
fs/coredump.c:dump_emit_page().  ->write_iter() (regular file or pipe one)
is called.  On ITER_BVEC.

It happens, and not only there.  Look at how /dev/loop works, for crying
out loud!  You get a struct request; the backing pages might very well have
_never_ been mapped to any userland address (e.g. when it's something like
a directory block).  And you hit this:

static int lo_write_simple(struct loop_device *lo, struct request *rq,
                loff_t pos)
{
        struct bio_vec bvec;
        struct req_iterator iter;
        int ret = 0;

        rq_for_each_segment(bvec, rq, iter) {
                ret = lo_write_bvec(lo->lo_backing_file, &bvec, &pos);
                if (ret < 0)
                        break;
                cond_resched();
        }

        return ret;
}

with lo_write_bvec() being

static int lo_write_bvec(struct file *file, struct bio_vec *bvec, loff_t *ppos)
{
        struct iov_iter i;
        ssize_t bw;

        iov_iter_bvec(&i, ITER_SOURCE, bvec, 1, bvec->bv_len);

        bw = vfs_iter_write(file, &i, ppos, 0);

        if (likely(bw ==  bvec->bv_len))
                return 0;   

        printk_ratelimited(KERN_ERR
                "loop: Write error at byte offset %llu, length %i.\n",
                (unsigned long long)*ppos, bvec->bv_len);
        if (bw >= 0)
                bw = -EIO;
        return bw;
}


Neither ->read_iter() nor ->write_iter() can make an assumption that it
will be used with userland buffer.  And yes, it works...