NetBSD Problem Report #54759

From www@netbsd.org  Thu Dec 12 22:06:40 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AEB747A14F
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 12 Dec 2019 22:06:40 +0000 (UTC)
Message-Id: <20191212220639.75F627A1E0@mollari.NetBSD.org>
Date: Thu, 12 Dec 2019 22:06:39 +0000 (UTC)
From: jaromir.dolecek@gmail.com
Reply-To: jaromir.dolecek@gmail.com
To: gnats-bugs@NetBSD.org
Subject: vm.ubc_direct deadlock when read()/write() into mapping of itself
X-Send-Pr-Version: www-1.0

>Number:         54759
>Category:       kern
>Synopsis:       vm.ubc_direct deadlock when read()/write() into mapping of itself
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    ad
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 12 22:10:00 +0000 2019
>Closed-Date:    Thu Apr 23 21:48:30 +0000 2020
>Last-Modified:  Thu Apr 23 21:50:01 +0000 2020
>Originator:     Jaromir Dolecek
>Release:        9.99.17
>Organization:
>Environment:
NetBSD pure-amd64 9.99.17 NetBSD 9.99.17 (QEMU-VIRT) #240: Sun Nov 10 01:40:13 CET 2019  dolecek@.../sys/arch/amd64/compile/QEMU-VIRT amd64
>Description:
ubc_direct code path locks buffer chage pages for the I/O when doing read()/write(). If the buffer passed to the I/O is actually a mmapped() buffer cache page, deadlock occurs. When this happens, the calling process is blocked, and also other processes fail to execute/finish.

This has been noted again in http://mail-index.netbsd.org/current-users/2019/12/03/msg037131.html and also in discussion when the code was introduced.

This problem needs to be fixed before we can turn the ubc_direct to on by default.
>How-To-Repeat:
1. sysctl -w vm.ubc_direct=1
2. compile and run attached program:

#include <sys/types.h>
#include <sys/uio.h>
#include <unistd.h>
#include <stdio.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <string.h>

int
main(int argc, const char **argv)
{
        FILE *fp;
        int fd;
        char buffer[8192];
        char buffer2[8192];

        fp = tmpfile();
        fd = fileno(fp);

        int rndfd = open("/dev/urandom", O_RDONLY);
        read(rndfd, buffer, sizeof(buffer));
        close(rndfd);

        fwrite(buffer, 1, sizeof(buffer), fp);
        fflush(fp);

        void *content;

        if ((content = mmap(NULL, sizeof(buffer), PROT_READ|PROT_WRITE, MAP_SHARED,
                fd, 0)) == NULL)
                perror("mmap");

        lseek(fd, 0, SEEK_SET);

        read(fd, buffer2, sizeof(buffer2));
        if (memcmp(buffer, buffer2, sizeof(buffer2)) != 0)
                fprintf(stderr, "memcmp mismatch\n");

        lseek(fd, 0, SEEK_SET);
        if (write(fd, content, sizeof(buffer)) < 0)
                perror("Write");
}
>Fix:
N/A

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->ad
Responsible-Changed-By: ad@NetBSD.org
Responsible-Changed-When: Thu, 09 Jan 2020 09:00:48 +0000
Responsible-Changed-Why:
I'll take a look.


From: Andrew Doran <ad@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/54759: vm.ubc_direct deadlock when read()/write() into
 mapping of itself
Date: Wed, 15 Jan 2020 20:03:43 +0000

 I can see how to prevent the deadlock (which I will try to do).  There is
 another problem though.  This goes ape bouncing around on PG_BUSY for
 libc.so's pages when doing build.sh on my machine.  I will try a bandaid
 with wakeup_one() but I think it needs PG_BUSY to be a kind of reader/writer
 lock to (something we could also use elsewhere).

 Andrew 

From: "Andrew Doran" <ad@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54759 CVS commit: src/sys/uvm
Date: Tue, 7 Apr 2020 19:11:13 +0000

 Module Name:	src
 Committed By:	ad
 Date:		Tue Apr  7 19:11:13 UTC 2020

 Modified Files:
 	src/sys/uvm: uvm_bio.c

 Log Message:
 PR kern/54759: vm.ubc_direct deadlock when read()/write() into mapping of itself

 Prevent ubc_uiomove_direct() on mapped vnodes.


 To generate a diff of this commit:
 cvs rdiff -u -r1.106 -r1.107 src/sys/uvm/uvm_bio.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: ad@NetBSD.org
State-Changed-When: Tue, 07 Apr 2020 19:21:01 +0000
State-Changed-Why:
This is fixed.  PG_BUSY still limits performance but that's a different issue.


State-Changed-From-To: closed->open
State-Changed-By: ad@NetBSD.org
State-Changed-When: Thu, 09 Apr 2020 23:14:23 +0000
State-Changed-Why:
The fix blocks ubc_direct on UAOs which disables it for tmpfs; not good enough.


State-Changed-From-To: open->closed
State-Changed-By: ad@NetBSD.org
State-Changed-When: Thu, 23 Apr 2020 21:48:30 +0000
State-Changed-Why:
okay this is definitely fixed now


From: "Andrew Doran" <ad@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54759 CVS commit: src/sys
Date: Thu, 23 Apr 2020 21:47:09 +0000

 Module Name:	src
 Committed By:	ad
 Date:		Thu Apr 23 21:47:09 UTC 2020

 Modified Files:
 	src/sys/fs/adosfs: advnops.c
 	src/sys/fs/cd9660: cd9660_vnops.c
 	src/sys/fs/efs: efs_vnops.c
 	src/sys/fs/filecorefs: filecore_vnops.c
 	src/sys/fs/hfs: hfs_vnops.c
 	src/sys/fs/msdosfs: msdosfs_denode.c msdosfs_vnops.c
 	src/sys/fs/nilfs: nilfs_vnops.c
 	src/sys/fs/puffs: puffs_vnops.c
 	src/sys/fs/sysvbfs: sysvbfs_vnops.c
 	src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c
 	src/sys/fs/udf: udf_allocation.c udf_vnops.c
 	src/sys/fs/v7fs: v7fs_vnops.c
 	src/sys/nfs: nfs_bio.c
 	src/sys/rump/librump/rumpvfs: rumpfs.c
 	src/sys/ufs/chfs: chfs_subr.c chfs_vnops.c
 	src/sys/ufs/ext2fs: ext2fs_inode.c ext2fs_readwrite.c
 	src/sys/ufs/ffs: ffs_inode.c
 	src/sys/ufs/lfs: lfs_inode.c ulfs_readwrite.c
 	src/sys/ufs/ufs: ufs_readwrite.c
 	src/sys/uvm: uvm_bio.c uvm_extern.h

 Log Message:
 PR kern/54759 (vm.ubc_direct deadlock when read()/write() into mapping of itself)

 - Add new flag UBC_ISMAPPED which tells ubc_uiomove() the object is mmap()ed
   somewhere.  Use it to decide whether to do direct-mapped copy, rather than
   poking around directly in the vnode in ubc_uiomove(), which is ugly and
   doesn't work for tmpfs.  It would be nicer to contain all this in UVM but
   the filesystem provides the needed locking here (VV_MAPPED) and to
   reinvent that would suck more.

 - Rename UBC_UNMAP_FLAG() to UBC_VNODE_FLAGS().  Pass in UBC_ISMAPPED where
   appropriate.


 To generate a diff of this commit:
 cvs rdiff -u -r1.52 -r1.53 src/sys/fs/adosfs/advnops.c
 cvs rdiff -u -r1.55 -r1.56 src/sys/fs/cd9660/cd9660_vnops.c
 cvs rdiff -u -r1.38 -r1.39 src/sys/fs/efs/efs_vnops.c
 cvs rdiff -u -r1.44 -r1.45 src/sys/fs/filecorefs/filecore_vnops.c
 cvs rdiff -u -r1.34 -r1.35 src/sys/fs/hfs/hfs_vnops.c
 cvs rdiff -u -r1.58 -r1.59 src/sys/fs/msdosfs/msdosfs_denode.c
 cvs rdiff -u -r1.101 -r1.102 src/sys/fs/msdosfs/msdosfs_vnops.c
 cvs rdiff -u -r1.39 -r1.40 src/sys/fs/nilfs/nilfs_vnops.c
 cvs rdiff -u -r1.214 -r1.215 src/sys/fs/puffs/puffs_vnops.c
 cvs rdiff -u -r1.64 -r1.65 src/sys/fs/sysvbfs/sysvbfs_vnops.c
 cvs rdiff -u -r1.108 -r1.109 src/sys/fs/tmpfs/tmpfs_subr.c
 cvs rdiff -u -r1.135 -r1.136 src/sys/fs/tmpfs/tmpfs_vnops.c
 cvs rdiff -u -r1.40 -r1.41 src/sys/fs/udf/udf_allocation.c
 cvs rdiff -u -r1.111 -r1.112 src/sys/fs/udf/udf_vnops.c
 cvs rdiff -u -r1.28 -r1.29 src/sys/fs/v7fs/v7fs_vnops.c
 cvs rdiff -u -r1.195 -r1.196 src/sys/nfs/nfs_bio.c
 cvs rdiff -u -r1.156 -r1.157 src/sys/rump/librump/rumpvfs/rumpfs.c
 cvs rdiff -u -r1.11 -r1.12 src/sys/ufs/chfs/chfs_subr.c
 cvs rdiff -u -r1.37 -r1.38 src/sys/ufs/chfs/chfs_vnops.c
 cvs rdiff -u -r1.88 -r1.89 src/sys/ufs/ext2fs/ext2fs_inode.c
 cvs rdiff -u -r1.76 -r1.77 src/sys/ufs/ext2fs/ext2fs_readwrite.c
 cvs rdiff -u -r1.127 -r1.128 src/sys/ufs/ffs/ffs_inode.c
 cvs rdiff -u -r1.159 -r1.160 src/sys/ufs/lfs/lfs_inode.c
 cvs rdiff -u -r1.26 -r1.27 src/sys/ufs/lfs/ulfs_readwrite.c
 cvs rdiff -u -r1.125 -r1.126 src/sys/ufs/ufs/ufs_readwrite.c
 cvs rdiff -u -r1.109 -r1.110 src/sys/uvm/uvm_bio.c
 cvs rdiff -u -r1.223 -r1.224 src/sys/uvm/uvm_extern.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search