fs: print warning when mount flags was ignored
PATCH 1/1 fs: print warning when mount flags was ignored
Some mount flags can conflict with each other so they can not be handled together. Currently when conflicting flags are specified, some of them are silently ignored putting user in believe that they was handled correctly.
Fix this by checking for unhandled flags and print warning when flags was ignored. I am not sure if it's worth translating the flags into the name, so it just prints the flag as a hexadecimal number.
Obviously we can not fail when conflicting flags are specified, because it may break buggy application, but it can change in the future.
--- fs/namespace.c | 24 +++++++++++++++++++----- 1 files changed, 19 insertions(+), 5 deletions(-)
fs/namespace.c +19 -5
@@ -2175,18 +2175,32 @@ long do_mount(char *dev_name, char *dir_name, char *type_page,MS_NOATIME |MS_NODIRATIME |MS_RELATIME |MS_KERNMOUNT | MS_STRICTATIME); -if (flags &MS_REMOUNT ) +if (flags &MS_REMOUNT ) { retval =do_remount (&path, flags & ~MS_REMOUNT , mnt_flags, data_page); -else if (flags &MS_BIND ) + flags &= ~MS_REMOUNT ; + }else if (flags &MS_BIND ) { retval =do_loopback (&path, dev_name, flags &MS_REC ); -else if (flags & (MS_SHARED |MS_PRIVATE |MS_SLAVE |MS_UNBINDABLE )) + flags &= ~MS_BIND ; + }else if (flags & (MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE )) { retval =do_change_type (&path, flags); -else if (flags &MS_MOVE ) + flags &= ~(MS_SHARED |MS_PRIVATE |MS_SLAVE |MS_UNBINDABLE ); + }else if (flags &MS_MOVE ) { retval =do_move_mount (&path, dev_name); -else + flags &= ~MS_MOVE ; + }else retval =do_new_mount (&path, type_page, flags, mnt_flags, dev_name, data_page); + + flags &= (MS_REMOUNT |MS_BIND |MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE |MS_MOVE ); + +if (!retval && flags) +printk (KERN_WARNING "%s(%u): (%s -> %s) Conflicting mount flags" +" specified. These flags has been " +"ignored: %#.8lx\n" , __func__, current->pid, + dev_name, dir_name, flags); dput_out:path_put (&path);return retval;
Signature · 1 line
1.7.4.4
Quoted 4 lines · Lukas Czerner
Some mount flags can conflict with each other so they can not be handled together. Currently when conflicting flags are specified, some of them are silently ignored putting user in believe that they was handled correctly.
Unfortunately, it's not so simple ;-)
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2181
Quoted 6 lines
fs/namespace.c +2
-2-if (flags &MS_REMOUNT ) +if (flags &MS_REMOUNT ) { retval =do_remount (&path, flags & ~MS_REMOUNT , mnt_flags, data_page); -else if (flags &MS_BIND ) + flags &= ~MS_REMOUNT ;
This is incorrect, the flags may also include many others flags. For example MS_REMOUNT|MS_BIND|MS_RDONLY is valid (see do_remoun() code).
And it's normal that for "mount -o remount" the mount command reads flags from mtab/fstab so it includes for example MS_RELATIME, ...
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2184
Quoted 4 lines
fs/namespace.c +2
-1+ }else if (flags &MS_BIND ) { retval =do_loopback (&path, dev_name, flags &MS_REC ); - else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE)) + flags &= ~MS_BIND ;
what about MS_REC ?
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2188
Quoted 5 lines
fs/namespace.c +3
-1+ }else if (flags & (MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE )) { retval =do_change_type (&path, flags); - else if (flags & MS_MOVE) + flags &= ~(MS_SHARED |MS_PRIVATE |MS_SLAVE |MS_UNBINDABLE );
what about MS_REC ?
Note that do_change_type() already checks for unexpected flags and returns -EINVAL if more flags are specified.
Quoted 6 lines
fs/namespace.c +3
-1+ }else if (flags &MS_MOVE ) { retval =do_move_mount (&path, dev_name); - else + flags &= ~MS_MOVE ; + }else retval = do_new_mount(&path, type_page, flags, mnt_flags,
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2203
Quoted 10 lines
fs/namespace.c +9
-0dev_name, data_page); + + flags &= (MS_REMOUNT |MS_BIND |MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE |MS_MOVE ); + +if (!retval && flags) +printk (KERN_WARNING "%s(%u): (%s -> %s) Conflicting mount flags" +" specified. These flags has been " +"ignored: %#.8lx\n" , __func__, current->pid, + dev_name, dir_name, flags);
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Quoted 8 lines · Karel Zak
Quoted 4 lines · Lukas Czerner
Some mount flags can conflict with each other so they can not be handled together. Currently when conflicting flags are specified, some of them are silently ignored putting user in believe that they was handled correctly.
Unfortunately, it's not so simple ;-)
Quoted 9 lines
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2181
Quoted 6 lines
fs/namespace.c +2
-2-if (flags &MS_REMOUNT ) +if (flags &MS_REMOUNT ) { retval =do_remount (&path, flags & ~MS_REMOUNT , mnt_flags, data_page); -else if (flags &MS_BIND ) + flags &= ~MS_REMOUNT ;This is incorrect, the flags may also include many others flags. For example MS_REMOUNT|MS_BIND|MS_RDONLY is valid (see
do_remoun()code).
ah, right we can specify more flags to be consumed by do_remount and others. Well, then we can just mask them all out like this:
flags &= ~(MS_REMOUNT | MS_BIND );
Quoted 3 lines
And it's normal that for "mount -o remount" the mount command reads flags from mtab/fstab so it includes for example
MS_RELATIME, ...
Yes, but MS_RELATIME is masked out from flags and it is used in mnt_flags which are handled by remount. That makes me wonder why we even have MS_RELATIME flag, since it is not used anywhere.
Quoted 7 lines
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2184
Quoted 5 lines
fs/namespace.c +2
-1+ }else if (flags &MS_BIND ) { retval =do_loopback (&path, dev_name, flags &MS_REC ); - else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE)) + flags &= ~MS_BIND ;what about
MS_REC?
Ok, we should put MS_REC into the mix since the do_remount does not use it.
flags &= ~(MS_BIND | MS_REC);
Quoted 8 lines
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2188
Quoted 6 lines
fs/namespace.c +3
-1+ }else if (flags & (MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE )) { retval =do_change_type (&path, flags); - else if (flags & MS_MOVE) + flags &= ~(MS_SHARED |MS_PRIVATE |MS_SLAVE |MS_UNBINDABLE );what about
MS_REC?
flags &= ~(MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE | MS_REC);
Quoted 3 lines
Note that
do_change_type()already checks for unexpected flags and returns-EINVALif more flags are specified.
I am not sure how this is related. Each of those function should probably do that but do_change_type() is actually the only one doing so.
Quoted 11 lines
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2197
Quoted 11 lines
fs/namespace.c +6
-1+ }else if (flags &MS_MOVE ) { retval =do_move_mount (&path, dev_name); - else + flags &= ~MS_MOVE ; + }else retval =do_new_mount (&path, type_page, flags, mnt_flags, dev_name, data_page); + + flags &= (MS_REMOUNT |MS_BIND |MS_SHARED |MS_PRIVATE | +MS_SLAVE |MS_UNBINDABLE |MS_MOVE );
flags &= (MS_REMOUNT | MS_BIND | MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE | MS_MOVE | MS_REC);
Quoted 6 lines
fs/namespace.c long do_mount(char *dev_name, char *dir_name, char *type_page, · line 2203
Quoted 6 lines
fs/namespace.c +6
-0+ +if (!retval && flags) +printk (KERN_WARNING "%s(%u): (%s -> %s) Conflicting mount flags" +" specified. These flags has been " +"ignored: %#.8lx\n" , __func__, current->pid, + dev_name, dir_name, flags);
But then, we'll might ignoring mnt_flags silently since they are masked out earlier. Well, maybe we could mas them out only do_remount() and do_new_mount().
Quoted 1 line
Karel
Thanks!
-Lukas