[Acked] [Raring PATCH 1/3] isofs: Refuse RW mount of the filesystem instead of making it RO

[Andy Whitcroft]

On Thu, Sep 26, 2013 at 02:40:09PM +0800, Hui Wang wrote:
> From: Jan Kara <jack at suse.cz>
> 
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1228751
> 
> Refuse RW mount of isofs filesystem. So far we just silently changed it
> to RO mount but when the media is writeable, block layer won't notice
> this change and thus will think device is used RW and will block eject
> button of the drive. That is unexpected by users because for
> non-writeable media eject button works just fine.
> 
> Userspace mount(8) command handles this just fine and retries mounting
> with MS_RDONLY set so userspace shouldn't see any regression.  Plus any
> tool mounting isofs is likely confronted with the case of read-only
> media where block layer already refuses to mount the filesystem without
> MS_RDONLY set so our behavior shouldn't be anything new for it.
> 
> Reported-by: Hui Wang <hui.wang at canonical.com>
> Signed-off-by: Jan Kara <jack at suse.cz>
> (cherry picked from commit 17b7f7cf58926844e1dd40f5eb5348d481deca6a)
> Signed-off-by: Hui Wang <hui.wang at canonical.com>
> ---
> After apply these 3 patches to raring kernel, i did a building test,
> the test passed. Then i used a dvd-rw isofs disc and cd-rw udf ro disc
> to test, they worked well and the eject button also work well.
> 
>  fs/isofs/inode.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 67ce525..6101c15 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -125,8 +125,8 @@ static void destroy_inodecache(void)
>  
>  static int isofs_remount(struct super_block *sb, int *flags, char *data)
>  {
> -	/* we probably want a lot more here */
> -	*flags |= MS_RDONLY;
> +	if (!(*flags & MS_RDONLY))
> +		return -EROFS;
>  	return 0;
>  }
>  
> @@ -779,15 +779,6 @@ root_found:
>  	 */
>  	s->s_maxbytes = 0x80000000000LL;
>  
> -	/*
> -	 * The CDROM is read-only, has no nodes (devices) on it, and since
> -	 * all of the files appear to be owned by root, we really do not want
> -	 * to allow suid.  (suid or devices will not show up unless we have
> -	 * Rock Ridge extensions)
> -	 */
> -
> -	s->s_flags |= MS_RDONLY /* | MS_NODEV | MS_NOSUID */;
> -
>  	/* Set this for reference. Its not currently used except on write
>  	   which we don't have .. */
>  
> @@ -1546,6 +1537,9 @@ struct inode *isofs_iget(struct super_block *sb,
>  static struct dentry *isofs_mount(struct file_system_type *fs_type,
>  	int flags, const char *dev_name, void *data)
>  {
> +	/* We don't support read-write mounts */
> +	if (!(flags & MS_RDONLY))
> +		return ERR_PTR(-EACCES);
>  	return mount_bdev(fs_type, flags, dev_name, data, isofs_fill_super);
>  }

The first and third patches are cherry-picks, the second is very very
minorly changed but seems sane.  Again we need this well tested in
-proposed pleased.

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw