linux-kernel - Re: [PATCH -next 1/2] nfs: nfs{,4}_file_flush should consume writeback error

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: <eab4bbb565a50bd09c2dbd3522177237fde2fad9.camel@hammerspace.com>
Date:   Sun, 6 Mar 2022 14:04:58 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "anna@...nel.org" <anna@...nel.org>,
        "chenxiaosong2@...wei.com" <chenxiaosong2@...wei.com>,
        "smayhew@...hat.com" <smayhew@...hat.com>
CC:     "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "liuyongqiang13@...wei.com" <liuyongqiang13@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "yi.zhang@...wei.com" <yi.zhang@...wei.com>,
        "zhangxiaoxu5@...wei.com" <zhangxiaoxu5@...wei.com>
Subject: Re: [PATCH -next 1/2] nfs: nfs{,4}_file_flush should consume
 writeback error

On Sun, 2022-03-06 at 11:54 +0800, chenxiaosong (A) wrote:
> It would be more clear if I update the reproducer like this:
> 
>          nfs server                 |       nfs client
>   --------------------------------- |--------------------------------
> -
>   # No space left on server         |
>   fallocate -l 100G /server/nospace |
>                                     | mount -t nfs $nfs_server_ip:/
> /mnt
>                                     |
>                                     | # Expected error
>                                     | dd if=/dev/zero of=/mnt/file
>                                     |
>                                     | # Release space on mountpoint
>                                     | rm /mnt/nospace
>                                     |
>                                     | # Unexpected error
>                                     | dd if=/dev/zero of=/mnt/file
> 
> The Unexpected error (No space left on device) when doing second
> `dd`, 
> is from unconsumed writeback error after close() the file when doing 
> first `dd`. There is enough space when doing second `dd`, we should
> not 
> report the nospace error.
> 
> We should report and consume the writeback error when userspace call 
> close()->flush(), the writeback error should not be left for next
> open().
> 
> Currently, fsync() will consume the writeback error while calling 
> file_check_and_advance_wb_err(), close()->flush() should also consume
> the writeback error.

No. That's not part of the API of any other filesystem. Why should we
make an exception for NFS?

The problem here seems to be the way that filemap_sample_wb_err() is
defined, and how it initialises f->f_wb_err in the function
do_dentry_open(). It is designed to do exactly what you see above.



-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@...merspace.com