[syzbot] WARNING in dma_map_sgtable (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 7e062cda7d90 Merge tag 'net-next-5.19' of git://git.kernel..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=172151d3f00000
kernel config: https://syzkaller.appspot.com/x/.config?x=e9d71d3c07c36588
dashboard link: https://syzkaller.appspot.com/bug?extid=3ba551855046ba3b3806
compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12918503f00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1386fa39f00000

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=14107ee5f00000
final oops: https://syzkaller.appspot.com/x/report.txt?x=16107ee5f00000
console output: https://syzkaller.appspot.com/x/log.txt?x=12107ee5f00000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3ba551...@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 0 PID: 3610 at kernel/dma/mapping.c:188 dma_map_sgtable+0x203/0x260 kernel/dma/mapping.c:264
Modules linked in:
CPU: 0 PID: 3610 Comm: syz-executor162 Not tainted 5.18.0-syzkaller-04943-g7e062cda7d90 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__dma_map_sg_attrs kernel/dma/mapping.c:188 [inline]
RIP: 0010:dma_map_sgtable+0x203/0x260 kernel/dma/mapping.c:264
Code: 75 15 e8 50 5f 14 00 eb cb e8 49 5f 14 00 eb c4 e8 42 5f 14 00 eb bd e8 3b 5f 14 00 0f 0b bd fb ff ff ff eb af e8 2d 5f 14 00 <0f> 0b 31 ed 48 bb 00 00 00 00 00 fc ff df e9 7b ff ff ff 89 e9 80
RSP: 0018:ffffc9000305fd40 EFLAGS: 00010293
RAX: ffffffff81723873 RBX: dffffc0000000000 RCX: ffff88801fbb8000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000002
RBP: ffff8881487e5408 R08: ffffffff81723743 R09: ffffed1003592c9e
R10: ffffed1003592c9e R11: 1ffff11003592c9c R12: ffff8881487e5000
R13: ffff88801ac964e0 R14: 0000000000000000 R15: 0000000000000001
FS: 0000555556c2a300(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000005d84c8 CR3: 000000001f1ef000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
get_sg_table+0xf9/0x150 drivers/dma-buf/udmabuf.c:72
begin_cpu_udmabuf+0xf5/0x160 drivers/dma-buf/udmabuf.c:126
dma_buf_begin_cpu_access+0xd8/0x170 drivers/dma-buf/dma-buf.c:1172
dma_buf_ioctl+0x2a0/0x2f0 drivers/dma-buf/dma-buf.c:363
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl+0xfb/0x170 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7f8bf9c6dc19
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffd7cfae1d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f8bf9c6dc19
RDX: 0000000020000100 RSI: 0000000040086200 RDI: 0000000000000006
RBP: 00007f8bf9c31dc0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f8bf9c31e50
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Hillf Danton]

On Mon, 30 May 2022 05:48:19 -0700

To queisce the warning, make udmabuf misc device dma capable by default.

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7e062cda7d90

--- y/drivers/dma-buf/udmabuf.c
+++ u/drivers/dma-buf/udmabuf.c
@@ -273,6 +273,14 @@ static long udmabuf_create(struct miscde
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto err;
+ } else {
+ struct device *dev = ubuf->device->this_device;
+
+ if (!dev->dma_mask) {
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
+ if (ret)
+ goto err;
+ }
}

flags = 0;
--

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+3ba551...@syzkaller.appspotmail.com

Tested on:

commit: 7e062cda Merge tag 'net-next-5.19' of git://git.kernel..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

kernel config: https://syzkaller.appspot.com/x/.config?x=e9d71d3c07c36588
dashboard link: https://syzkaller.appspot.com/bug?extid=3ba551855046ba3b3806
compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=137777cbf00000

Note: testing is done by a robot and is best-effort only.

[Dan Carpenter]

On Mon, May 30, 2022 at 10:45:42PM +0800, Hillf Danton wrote:
> To queisce the warning, make udmabuf misc device dma capable by default.
>
> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7e062cda7d90
>
> --- y/drivers/dma-buf/udmabuf.c
> +++ u/drivers/dma-buf/udmabuf.c
> @@ -273,6 +273,14 @@ static long udmabuf_create(struct miscde
> if (IS_ERR(buf)) {
> ret = PTR_ERR(buf);
> goto err;
> + } else {
> + struct device *dev = ubuf->device->this_device;
> +
> + if (!dev->dma_mask) {
> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (ret)
> + goto err;
> + }
> }
>
> flags = 0;

Unrelated to your patch. In this function do we not need to cleanup if
dma_buf_fd(buf, flags); fails?

After dma_buf_fd() then the fd is exported so we can't clean up, but it
should be okay to clean up if dma_buf_fd() fails.

regards,
dan carpenter

[Hillf Danton]

On Mon, 30 May 2022 18:10:44 +0300 Dan Carpenter wrote:
>
> Unrelated to your patch. In this function do we not need to cleanup if
> dma_buf_fd(buf, flags); fails?

Thanks for taking a look.

Rollback in genernel is needed in case of error as you tip point.

>
> After dma_buf_fd() then the fd is exported so we can't clean up, but it
> should be okay to clean up if dma_buf_fd() fails.

Care to post a diff to specify the points in your mind?

Hillf

[Christoph Hellwig]

Someohow only the reply from Dan got delivered to me, not the mail
from Hillf he is replying to, so I'm abusing that to reply to the
previous mail..

On Mon, May 30, 2022 at 06:10:44PM +0300, Dan Carpenter wrote:
> > --- y/drivers/dma-buf/udmabuf.c
> > +++ u/drivers/dma-buf/udmabuf.c
> > @@ -273,6 +273,14 @@ static long udmabuf_create(struct miscde
> > if (IS_ERR(buf)) {
> > ret = PTR_ERR(buf);
> > goto err;
> > + } else {
> > + struct device *dev = ubuf->device->this_device;
> > +
> > + if (!dev->dma_mask) {
> > + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > + if (ret)
> > + goto err;
> > + }
> > }

This is compeltely broken. If the underlying device is ot DMA capable and
we can't just set a random mask and still allow DMA mappings.

[Hillf Danton]

On Mon, 30 May 2022 22:45:34 -0700 Christoph Hellwig wrote:
>
> This is compeltely broken. If the underlying device is ot DMA capable and
> we can't just set a random mask and still allow DMA mappings.

You are right. Thanks for taking a look.

And IMO we can do less on the exporter side as the udmabuf misc device
is different from those that for instance are dma capable on the PCI bus.
Regular dma games go only on the importer side.

--- y/drivers/dma-buf/udmabuf.c
+++ u/drivers/dma-buf/udmabuf.c
@@ -55,7 +55,7 @@ static int mmap_udmabuf(struct dma_buf *
}

static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
- enum dma_data_direction direction)
+ enum dma_data_direction direction, int mapping)
{
struct udmabuf *ubuf = buf->priv;
struct sg_table *sg;
@@ -69,6 +69,8 @@ static struct sg_table *get_sg_table(str
GFP_KERNEL);
if (ret < 0)
goto err;
+ if (!mapping)
+ return sg;
ret = dma_map_sgtable(dev, sg, direction, 0);
if (ret < 0)
goto err;
@@ -91,7 +93,7 @@ static void put_sg_table(struct device *
static struct sg_table *map_udmabuf(struct dma_buf_attachment *at,
enum dma_data_direction direction)
{
- return get_sg_table(at->dev, at->dmabuf, direction);
+ return get_sg_table(at->dev, at->dmabuf, direction, 1);
}

static void unmap_udmabuf(struct dma_buf_attachment *at,
@@ -123,14 +125,10 @@ static int begin_cpu_udmabuf(struct dma_

struct device *dev = ubuf->device->this_device;

if (!ubuf->sg) {
- ubuf->sg = get_sg_table(dev, buf, direction);
+ ubuf->sg = get_sg_table(dev, buf, direction, 0);
if (IS_ERR(ubuf->sg))
return PTR_ERR(ubuf->sg);
- } else {
- dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents,
- direction);
}
-
return 0;
}

@@ -138,12 +136,14 @@ static int end_cpu_udmabuf(struct dma_bu
enum dma_data_direction direction)
{
struct udmabuf *ubuf = buf->priv;
+ /*
+ * keep dev intact, thanks.
+ *

struct device *dev = ubuf->device->this_device;

+ */

if (!ubuf->sg)
return -EINVAL;
-
- dma_sync_sg_for_device(dev, ubuf->sg->sgl, ubuf->sg->nents, direction);
return 0;
}

[Christoph Hellwig]

On Tue, May 31, 2022 at 05:04:18PM +0800, Hillf Danton wrote:
> You are right. Thanks for taking a look.
>
> And IMO we can do less on the exporter side as the udmabuf misc device
> is different from those that for instance are dma capable on the PCI bus.
> Regular dma games go only on the importer side.

I think you need the relevant maintainers here, this is above my
paygrade.