linux-kernel - Re: [patch] direct-io: propagate -ENOSPC errors

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: <x491t7360p3.fsf@segfault.boston.devel.redhat.com>
Date:	Mon, 21 Mar 2016 16:22:16 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch] direct-io: propagate -ENOSPC errors

Hi, Christoph,

Christoph Hellwig <hch@...radead.org> writes:

> On Mon, Mar 14, 2016 at 05:10:00PM -0400, Jeff Moyer wrote:
>> dio_bio_complete turns all errors into -EIO.  This is historical,
>> since you used to only get 1 bit precision for errors (BIO_UPTODATE).
>> Now that we get actual error codes, we can return the appropriate
>> code to userspace.  File systems seem to only propagate either EIO
>> or ENOSPC, so I've followed suit in this patch.
>
> Do we?

Sometimes!  :)  I was referring to this:

static inline void mapping_set_error(struct address_space *mapping, int error)
{
        if (unlikely(error)) {
                if (error == -ENOSPC)
                        set_bit(AS_ENOSPC, &mapping->flags);
                else
                        set_bit(AS_EIO, &mapping->flags);
        }
}

> Just propagating some errors defintively seems odd.

Not really.  read, write, etc only expect a subset of errnos to be
returned.  The goal was not to leak kernel-internal or unexpected error
numbers to userspace, and I didn't think I would be able to successfully
audit all code paths that lead here.  So, I opted for a more
conservative patch that just allows one more errno through.

> Even if we do we should have a central helper doing that mapping
> instead of opencoding it in various places.

OK.  I would argue that the right place to filter out errno's would be
up in the vfs.  I do wonder if I'm just being overly paranoid, though.

Al, do you have any opinions, here?

Cheers,
Jeff