linux-kernel - Re: [git pull] first batch of ufs fixes

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:   Sun, 18 Jun 2017 13:45:41 -0700 (PDT)
From:   Richard Narron <comet.berkeley@...il.com>
To:     Al Viro <viro@...IV.linux.org.uk>
cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] first batch of ufs fixes

On Sun, 18 Jun 2017, Al Viro wrote:

> On Sat, Jun 17, 2017 at 03:15:48AM +0100, Al Viro wrote:
>> On Fri, Jun 16, 2017 at 07:29:00AM -0700, Richard Narron wrote:
>>
>>> The 8 patches in the ufs-fixes group were applied to Linux 4.12-rc5.
>>> They seem to work fine with the simple testing that I do.
>>>
>>> I tested all 3 BSDs, FreeBSD 11.0, OpenBSD 6.1 and NetBSD 7.1 using 2
>>> filesystems, 44bsd (ufs1) and ufs2.
>>> I found no errors doing a Linux mkdir, copy large file, BSD fsck, Linux rm
>>> large file, rmdir and BSD fsck in any of the 6 combinations.
>>
>> FWIW, with xfstests I see the following:
>> 	* _require_metadata_journaling needs to be told not to bother with
>> our ufs
>> 	* generic/{409,410,411} lack $MOUNT_OPTIONS in several invocations
>> of mount -t $FSTYP; for ufs it's obviosuly a problem.  Trivial bug in tests,
>> fixing it makes them pass.
>> 	* generic/258 (timestamp wraparound) fails; fs is left intact
>
> Trivially fixed (cast to (signed) in ufs1_read_inode(), similar to what
> other filesystems with 32bit timestamps are doing); ufs2 has no problem
> at all)
>
>> 	* generic/426 (fhandle stuff) fails and buggers the filesystem
>> Everything else passes with no fs corruption that could be detected by
>> fsck.ufs.
>
> Also trivially fixed - it's a self-inflicted wound.  Just have zero nlink in
> ufs{1,2}_read_inode() fail with -ESTALE instead of triggering ufs_error().
>
>> As for my immediate plans, I'll look into the two failing tests,
>> but any further active work on ufs will have to wait for the next
>> cycle.  It had been a fun couple of weeks, but I have more than
>> enough other stuff to deal with.  And I would still very much prefer
>> for somebody to adopt that puppy.
>
> Another piece of fun spotted: the logics for switching between two allocation
> policies when relocating a packed tail that can't be expanded in place had
> been b0rken since typo in 2.4.14.7 - switch back from OPTTIME to OPTSPACE
> had been screwed by this:
> -               usb1->fs_optim = SWAB32(UFS_OPTSPACE);
> +               usb1->fs_optim = cpu_to_fs32(sb, UFS_OPTTIME);
>
> And fragmentation levels for switching back and force really ought to be
> calculated at mount time.  Another (minor) issue is mentioned in this
> commit message from Kirck McKusick back in 1995:
>        The threshold for switching from time-space and space-time is too small
>        when minfree is 5%...so make it stay at space in this case.
> Not that minfree at 5% had been frequently seen - default has never been that
> low (back in 4.2BSD it was 10%, these days it's 8%)
>
> Resulting kernel passes xfstests clean and now I'm definitely done with UFS for
> this cycle.  Linus, in case you want to pull that sucker, pull request would
> be as below:
>
> The following changes since commit a8fad984833832d5ca11a9ed64ddc55646da30e3:
>
>  ufs_truncate_blocks(): fix the case when size is in the last direct block (2017-06-15 03:57:46 -0400)
>
> are available in the git repository at:
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git ufs-fixes
>
> for you to fetch changes up to 77e9ce327d9b607cd6e57c0f4524a654dc59c4b1:
>
>  ufs: fix the logics for tail relocation (2017-06-17 17:22:42 -0400)
>
> ----------------------------------------------------------------
> Al Viro (3):
>      fix signedness of timestamps on ufs1
>      ufs_iget(): fail with -ESTALE on deleted inode
>      ufs: fix the logics for tail relocation
>
> fs/ufs/balloc.c | 22 ++++++----------------
> fs/ufs/inode.c  | 27 +++++++++++----------------
> fs/ufs/super.c  |  9 +++++++++
> fs/ufs/ufs_fs.h |  2 ++
> 4 files changed, 28 insertions(+), 32 deletions(-)
>

I just tested these 3 patches along with the earlier 8 patches against 
Linux 4.12-rc5 and they look fine. All 6 test cases look good.

The ufs code is much better now than it was before all these patches.