linux-kernel - [Q] is the amount of residual bytes still not guaranteed for to be available for some old SCSI drivers? (was Re: Update bisect info and new repro code for "[syzbot] WARNING in _copy_from

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] [day] [month] [year] [list]

Message-ID: <Y7ZGRy4C4MMeWIwA@ZenIV>
Date:   Thu, 5 Jan 2023 03:38:47 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Pengfei Xu <pengfei.xu@...el.com>
Cc:     heng.su@...el.com, linux-kernel@...r.kernel.org,
        davem@...emloft.net, jmaloy@...hat.com, kuba@...nel.org,
        linux-scsi@...r.kernel.org, linux-block@...r.kernel.org
Subject: [Q] is the amount of residual bytes still not guaranteed for to be
 available for some old SCSI drivers? (was Re: Update bisect info and new
 repro code for "[syzbot] WARNING in _copy_from_iter")

On Thu, Jan 05, 2023 at 09:48:10AM +0800, Pengfei Xu wrote:
> Hi Viro,
> 
> It's a soft remind: "_copy_from_iter" WARNING issue was still reproduced
> in v6.2-rc2 mainline kernel in guest.

sorry, had been sick ;-/

I see what's going on there; it's this bit:

        if ((iov_iter_rw(iter) == WRITE &&
             (!map_data || !map_data->null_mapped)) ||  
            (map_data && map_data->from_user)) {
                ret = bio_copy_from_iter(bio, iter);   
                if (ret)
                        goto cleanup;

The "map_data && map_data->from_user" part is what I'd missed.
Mea culpa.

That code really is reading from destination here; in a lot of
respects that's broken (consider the case when destination is
mapped write-only), but it's a deliberate behaviour.

Origin is in commit ecb554a846f8 "block: fix sg SG_DXFER_TO_FROM_DEV
regression".  Before trying to kludge around that, how much of the
original problem is still there?

Are there still low-level drivers that don't bother with scsi_set_resid()
for passthrough requests?  IOW, do we need to play with
	copy the entire destination into kernel buffer
	handle REQ_OP_DRV_IN request, overwriting the copy
	copy the entire buffer back to destination
all because we can't tell how much data had been copied in?
Because if we had the length of that sucker in sg_rq_end_io(),
we could just arrange for bio->bmd->iter truncated to actual
amount read and that would be it.

NOTE: no other users of blk_rq_map_user_io() go anywhere near that
weirdness; it's really just /dev/sg*.

If we that's approach is not feasible, we can always deal with
that in bio_copy_from_iter() - forcibly flip ->data_source for the
duration, which is OK for all kinds of iov_iter that can reach that
thing; it would work, but that's really ugly ;-/  If there's really
no way to get the amount of data actually transferred, that's what
we'll have to do, but it would be much saner to avoid the entire
song and dance and just copy the right amount out...