[PATCH] init/do_mounts.c: treat EROFS like EACCES

[PATCH] init/do_mounts.c: treat EROFS like EACCES

[Philippe De Muyter]

some combinations of filesystem and block device (at least vfat on mmc)
yield -EROFS instead of -EACCES when the device is read-only.  Retry
mounting with MS_RDONLY set, just like for the EACCES case, instead of
failing directly.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 init/do_mounts.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 82f2288..af7b9b8 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -394,6 +394,7 @@ retry:
 			case 0:
 				goto out;
 			case -EACCES:
+			case -EROFS:
 				flags |= MS_RDONLY;
 				goto retry;
 			case -EINVAL:
-- 
1.7.5.3

Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES

[Andrew Morton]

On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter <phdm@macqel.be> wrote:

> some combinations of filesystem and block device (at least vfat on mmc)
> yield -EROFS instead of -EACCES when the device is read-only.  Retry
> mounting with MS_RDONLY set, just like for the EACCES case, instead of
> failing directly.
> 
> ...
>
> --- a/init/do_mounts.c
> +++ b/init/do_mounts.c
> @@ -394,6 +394,7 @@ retry:
>  			case 0:
>  				goto out;
>  			case -EACCES:
> +			case -EROFS:
>  				flags |= MS_RDONLY;
>  				goto retry;
>  			case -EINVAL:

hm, what's going on here.  I'd have thought it to be very logical that
file_system_type.mount() would return EROFS if the device is read-only!
But I'm suspecting that there is some convention that the fs is
supposed to return EACCES in this case.  So *perhaps* it is vfat-on-mmc
which needs fixing.  Dunno.

Al, are you able to shed light?

Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES

[Dave Chinner]

On Thu, Jun 19, 2014 at 02:19:50PM -0700, Andrew Morton wrote:
> On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> 
> > some combinations of filesystem and block device (at least vfat on mmc)
> > yield -EROFS instead of -EACCES when the device is read-only.  Retry
> > mounting with MS_RDONLY set, just like for the EACCES case, instead of
> > failing directly.
> > 
> > ...
> >
> > --- a/init/do_mounts.c
> > +++ b/init/do_mounts.c
> > @@ -394,6 +394,7 @@ retry:
> >  			case 0:
> >  				goto out;
> >  			case -EACCES:
> > +			case -EROFS:
> >  				flags |= MS_RDONLY;
> >  				goto retry;
> >  			case -EINVAL:
> 
> hm, what's going on here.  I'd have thought it to be very logical that
> file_system_type.mount() would return EROFS if the device is read-only!
> But I'm suspecting that there is some convention that the fs is
> supposed to return EACCES in this case.  So *perhaps* it is vfat-on-mmc
> which needs fixing.  Dunno.
> 
> Al, are you able to shed light?

from the mount(2) man page:

EACCES	A  component  of  a  path  was not searchable.  (See also
	path_resolution(7).)  Or, mounting a read-only filesystem
	was attempted without giving the MS_RDONLY flag.  Or, the
	block device source is located on a filesystem mounted with
	the MS_NODEV option.

So, when the device is read-only, the error should EACCES, not
EROFS. Would seem to me that vfat-on-mmc needs fixing...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH] VFS: mount must return EACCES, not EROFS

[Philippe De Muyter]

mount must return EACCES, not EROFS, when one attempts to mount a
read-only filesystem in read-write mode, but the file-system layer
only transmits the error given by the block layer, and many block
drivers return EROFS in that case, so let's fix it in do_mount.

Actually it is only a small problem for a user using the mount(1)
command, because EROFS is actually a more explicit answer than
EACCES, but init/do_mounts.c checks only for EACCES, not EROFS,
to decide to retry to mount the root file-system in read-only mode,
and so we are left with an unbootable kernel, and with a cryptic
error message (*) if the root partition happens to be read-only

(*): VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2):
error -30

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/namespace.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 182bc41..6291a3f 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char *dir_name,
 
 	retval = security_sb_mount(dev_name, &path,
 				   type_page, flags, data_page);
+	if (retval == -EROFS)
+		retval = -EACCES;
 	if (!retval && !may_mount())
 		retval = -EPERM;
 	if (retval)
-- 
1.7.5.3

Re: [PATCH PING] VFS: mount must return EACCES, not EROFS

[Philippe De Muyter]

PING

Currently, the initial mount of the root file system by the linux
kernel fails with a cryptic message instead of being retried with
the MS_RDONLY flag set,  when the device is read-only and the
combination of block driver and filesystem driver yields EROFS.

I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
if linux aims to strict compliance with POSIX on that point.  Consensus
amongst the messages that I have read so far seems to show that linux
kernel hackers feel that EROFS is a more appropriate error code than
EACCES in that case.

So, do you choose for my first pragmatic and non-intrusive patch, that
lets mount_block_root() retry with MS_RDONLY if the file system
returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second
one that forces all file-systems to return EACCES instead of EROFS.
(https://lkml.org/lkml/2014/6/20/98).

Best regards

Philippe

On Fri, Jun 20, 2014 at 10:39:22AM +0200, Philippe De Muyter wrote:
> mount must return EACCES, not EROFS, when one attempts to mount a
> read-only filesystem in read-write mode, but the file-system layer
> only transmits the error given by the block layer, and many block
> drivers return EROFS in that case, so let's fix it in do_mount.
> 
> Actually it is only a small problem for a user using the mount(1)
> command, because EROFS is actually a more explicit answer than
> EACCES, but init/do_mounts.c checks only for EACCES, not EROFS,
> to decide to retry to mount the root file-system in read-only mode,
> and so we are left with an unbootable kernel, and with a cryptic
> error message (*) if the root partition happens to be read-only
> 
> (*): VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2):
> error -30
> 
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Dave Chinner <david@fromorbit.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-fsdevel@vger.kernel.org
> ---
>  fs/namespace.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 182bc41..6291a3f 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char *dir_name,
>  
>  	retval = security_sb_mount(dev_name, &path,
>  				   type_page, flags, data_page);
> +	if (retval == -EROFS)
> +		retval = -EACCES;
>  	if (!retval && !may_mount())
>  		retval = -EPERM;
>  	if (retval)
> -- 
> 1.7.5.3

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH PING] VFS: mount must return EACCES, not EROFS

[Andrew Morton]

On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter <phdm@macqel.be> wrote:

> Currently, the initial mount of the root file system by the linux
> kernel fails with a cryptic message instead of being retried with
> the MS_RDONLY flag set,  when the device is read-only and the
> combination of block driver and filesystem driver yields EROFS.
> 
> I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
> if linux aims to strict compliance with POSIX on that point.  Consensus
> amongst the messages that I have read so far seems to show that linux
> kernel hackers feel that EROFS is a more appropriate error code than
> EACCES in that case.
> 
> So, do you choose for my first pragmatic and non-intrusive patch, that
> lets mount_block_root() retry with MS_RDONLY if the file system
> returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second
> one that forces all file-systems to return EACCES instead of EROFS.
> (https://lkml.org/lkml/2014/6/20/98).

They both seem a little hacky to me.

Isn't the core problem that "the combination of block driver and
filesystem driver yields EROFS"?  That the fs should instead be
returning EACCESS in this case?

What fs and block driver are we talking about here, anyway?

Re: [PATCH PING] VFS: mount must return EACCES, not EROFS

[Philippe De Muyter]

On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote:
> On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> 
> > Currently, the initial mount of the root file system by the linux
> > kernel fails with a cryptic message instead of being retried with
> > the MS_RDONLY flag set,  when the device is read-only and the
> > combination of block driver and filesystem driver yields EROFS.
> > 
> > I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
> > if linux aims to strict compliance with POSIX on that point.  Consensus
> > amongst the messages that I have read so far seems to show that linux
> > kernel hackers feel that EROFS is a more appropriate error code than
> > EACCES in that case.
> 
> Isn't the core problem that "the combination of block driver and
> filesystem driver yields EROFS"?  That the fs should instead be
> returning EACCESS in this case?

Does POSIX or Linux mandate that it should ?

> 
> What fs and block driver are we talking about here, anyway?

The problem happened to me with a f2fs filesystem on a sd-card that was
accidentally write-protected and that was put in a SD-card slot (mmc block
driver).

I retested using mount(8) with a similar now intentionnaly write-protected
sd card in a usb reader (usb_storage driver ?) with vfat, f2fs and ext4
filesystems with the following results :

  mywdesk:~ # strace -e mount mount /dev/sdb1 /mnt
  mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
  mount: /dev/sdb1 is write-protected, mounting read-only
  mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL|MS_RDONLY, NULL) = 0
  +++ exited with 0 +++
  mywdesk:~ # umount /mnt
  mywdesk:~ # strace -e mount mount -t f2fs /dev/sdb2 /mnt
  mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
  mount: /dev/sdb2 is write-protected, mounting read-only
  mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL|MS_RDONLY, NULL) = 0
  +++ exited with 0 +++
  mywdesk:~ # umount /mnt
  mywdesk:~ # strace -e mount mount /dev/sdb3 /mnt
  mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
  mount: /dev/sdb3 is write-protected, mounting read-only
  mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL|MS_RDONLY, NULL) = 0
  +++ exited with 0 +++
  mywdesk:~ #

All three file-systems (vfat, f2fs & ext4) yield EROFS.

I also quickly grepped for occurences of EROFS under fs/, and found no check
to replace EROFS by EACCES,
while the same grep under drivers/{block,cdrom,ide,md,memstick, mtd,
s390/block,scsi,usb} gives plenty of "return -EROFS;"

So, if no filesystem driver replaces EROFS by EACCES and many block drivers
return EROFS, it seems to me that many combinations will yield EROFS.

> > 
> > So, do you choose for my first pragmatic and non-intrusive patch, that
> > lets mount_block_root() retry with MS_RDONLY if the file system
> > returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second
> > one that forces all file-systems to return EACCES instead of EROFS.
> > (https://lkml.org/lkml/2014/6/20/98).
> 
> They both seem a little hacky to me.

Actually I prefer my first patch, which simply adapts the kernel to the current
situation, like mount(8) already does, instead of trying to impose an ABI
change.

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH PING] VFS: mount must return EACCES, not EROFS

[Andrew Morton]

On Thu, 3 Jul 2014 18:29:19 +0200 Philippe De Muyter <phdm@macqel.be> wrote:

> On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote:
> > On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> > 
> > > Currently, the initial mount of the root file system by the linux
> > > kernel fails with a cryptic message instead of being retried with
> > > the MS_RDONLY flag set,  when the device is read-only and the
> > > combination of block driver and filesystem driver yields EROFS.
> > > 
> > > I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
> > > if linux aims to strict compliance with POSIX on that point.  Consensus
> > > amongst the messages that I have read so far seems to show that linux
> > > kernel hackers feel that EROFS is a more appropriate error code than
> > > EACCES in that case.
> > 
> > Isn't the core problem that "the combination of block driver and
> > filesystem driver yields EROFS"?  That the fs should instead be
> > returning EACCESS in this case?
> 
> Does POSIX or Linux mandate that it should ?
> 
> > 
> > What fs and block driver are we talking about here, anyway?
> 
> The problem happened to me with a f2fs filesystem on a sd-card that was
> accidentally write-protected and that was put in a SD-card slot (mmc block
> driver).
> 
> I retested using mount(8) with a similar now intentionnaly write-protected
> sd card in a usb reader (usb_storage driver ?) with vfat, f2fs and ext4
> filesystems with the following results :
> 
>   mywdesk:~ # strace -e mount mount /dev/sdb1 /mnt
>   mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
>   mount: /dev/sdb1 is write-protected, mounting read-only
>   mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL|MS_RDONLY, NULL) = 0
>   +++ exited with 0 +++
>   mywdesk:~ # umount /mnt
>   mywdesk:~ # strace -e mount mount -t f2fs /dev/sdb2 /mnt
>   mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
>   mount: /dev/sdb2 is write-protected, mounting read-only
>   mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL|MS_RDONLY, NULL) = 0
>   +++ exited with 0 +++
>   mywdesk:~ # umount /mnt
>   mywdesk:~ # strace -e mount mount /dev/sdb3 /mnt
>   mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL, NULL) = -1 EROFS (Read-only file system)
>   mount: /dev/sdb3 is write-protected, mounting read-only
>   mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL|MS_RDONLY, NULL) = 0
>   +++ exited with 0 +++
>   mywdesk:~ #
> 
> All three file-systems (vfat, f2fs & ext4) yield EROFS.
> 
> I also quickly grepped for occurences of EROFS under fs/, and found no check
> to replace EROFS by EACCES,
> while the same grep under drivers/{block,cdrom,ide,md,memstick, mtd,
> s390/block,scsi,usb} gives plenty of "return -EROFS;"
> 
> So, if no filesystem driver replaces EROFS by EACCES and many block drivers
> return EROFS, it seems to me that many combinations will yield EROFS.

hm.  I'm thinking that in an ideal world, those block drivers will
return -EACCES rather than -EROFS.  You open() a read-only device for
rw, you get "permission denied".

Al, speak to us.

Re: [PATCH PING] VFS: mount must return EACCES, not EROFS

[Philippe De Muyter]

On Tue, Jul 08, 2014 at 02:02:18PM -0700, Andrew Morton wrote:
> On Thu, 3 Jul 2014 18:29:19 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> 
> > On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote:
> > > On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> > > 
> > > > Currently, the initial mount of the root file system by the linux
> > > > kernel fails with a cryptic message instead of being retried with
> > > > the MS_RDONLY flag set,  when the device is read-only and the
> > > > combination of block driver and filesystem driver yields EROFS.
> > > > 
> > > > I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
> > > > if linux aims to strict compliance with POSIX on that point.  Consensus
> > > > amongst the messages that I have read so far seems to show that linux
> > > > kernel hackers feel that EROFS is a more appropriate error code than
> > > > EACCES in that case.
> > > 
> > > Isn't the core problem that "the combination of block driver and
> > > filesystem driver yields EROFS"?  That the fs should instead be
> > > returning EACCESS in this case?
> > 
> > Does POSIX or Linux mandate that it should ?
> > 

For info, SCO Unix documents that mount(2) may fail with EROFS :
and adds "mount is not part of any currently supported standard"

	http://osr507doc.sco.com/en/man/html.S/mount.S.html

Philippe

Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES

[Philippe De Muyter]

On Fri, Jun 20, 2014 at 09:09:24AM +1000, Dave Chinner wrote:
> On Thu, Jun 19, 2014 at 02:19:50PM -0700, Andrew Morton wrote:
> > On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> > 
> > > some combinations of filesystem and block device (at least vfat on mmc)
> > > yield -EROFS instead of -EACCES when the device is read-only.  Retry
> > > mounting with MS_RDONLY set, just like for the EACCES case, instead of
> > > failing directly.
> > > 
> > > ...
> > >
> > > --- a/init/do_mounts.c
> > > +++ b/init/do_mounts.c
> > > @@ -394,6 +394,7 @@ retry:
> > >  			case 0:
> > >  				goto out;
> > >  			case -EACCES:
> > > +			case -EROFS:
> > >  				flags |= MS_RDONLY;
> > >  				goto retry;
> > >  			case -EINVAL:
> > 
> > hm, what's going on here.  I'd have thought it to be very logical that
> > file_system_type.mount() would return EROFS if the device is read-only!
> > But I'm suspecting that there is some convention that the fs is
> > supposed to return EACCES in this case.  So *perhaps* it is vfat-on-mmc
> > which needs fixing.  Dunno.
> > 
> > Al, are you able to shed light?
> 
> from the mount(2) man page:
> 
> EACCES	A  component  of  a  path  was not searchable.  (See also
> 	path_resolution(7).)  Or, mounting a read-only filesystem
> 	was attempted without giving the MS_RDONLY flag.  Or, the
> 	block device source is located on a filesystem mounted with
> 	the MS_NODEV option.
> 
> So, when the device is read-only, the error should EACCES, not
> EROFS. Would seem to me that vfat-on-mmc needs fixing...

Looking at the sources of mount(1)

https://github.com/karelzak/util-linux/blob/master/sys-utils/mount.c

at line 601, we clearly see that mount(1) allows mount(2) to fail
with EROFS.

We could as well fix the man page of mount(2)

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES

[Karel Zak]

On Fri, Jun 20, 2014 at 03:13:28PM +0200, Philippe De Muyter wrote:
> On Fri, Jun 20, 2014 at 09:09:24AM +1000, Dave Chinner wrote:
> > On Thu, Jun 19, 2014 at 02:19:50PM -0700, Andrew Morton wrote:
> > > On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter <phdm@macqel.be> wrote:
> > > 
> > > > some combinations of filesystem and block device (at least vfat on mmc)
> > > > yield -EROFS instead of -EACCES when the device is read-only.  Retry
> > > > mounting with MS_RDONLY set, just like for the EACCES case, instead of
> > > > failing directly.
> > > > 
> > > > ...
> > > >
> > > > --- a/init/do_mounts.c
> > > > +++ b/init/do_mounts.c
> > > > @@ -394,6 +394,7 @@ retry:
> > > >  			case 0:
> > > >  				goto out;
> > > >  			case -EACCES:
> > > > +			case -EROFS:
> > > >  				flags |= MS_RDONLY;
> > > >  				goto retry;
> > > >  			case -EINVAL:
> > > 
> > > hm, what's going on here.  I'd have thought it to be very logical that
> > > file_system_type.mount() would return EROFS if the device is read-only!
> > > But I'm suspecting that there is some convention that the fs is
> > > supposed to return EACCES in this case.  So *perhaps* it is vfat-on-mmc
> > > which needs fixing.  Dunno.
> > > 
> > > Al, are you able to shed light?
> > 
> > from the mount(2) man page:
> > 
> > EACCES	A  component  of  a  path  was not searchable.  (See also
> > 	path_resolution(7).)  Or, mounting a read-only filesystem
> > 	was attempted without giving the MS_RDONLY flag.  Or, the
> > 	block device source is located on a filesystem mounted with
> > 	the MS_NODEV option.
> > 
> > So, when the device is read-only, the error should EACCES, not
> > EROFS. Would seem to me that vfat-on-mmc needs fixing...

Only code matters, we don't compile and execute man pages... ;-)

> Looking at the sources of mount(1)
> 
> https://github.com/karelzak/util-linux/blob/master/sys-utils/mount.c
> 
> at line 601, we clearly see that mount(1) allows mount(2) to fail
> with EROFS.

BTW, comment from the original mount(8) code:

     case EACCES:  /* pre-linux 1.1.38, 1.1.41 and later */
     case EROFS:   /* linux 1.1.38 and later */

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com