diff mbox series

tomoyo: Change pathname calculation for read-only filesystems.

Message ID 1551277164-8715-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive)
State New, archived
Headers show
Series tomoyo: Change pathname calculation for read-only filesystems. | expand

Commit Message

Tetsuo Handa Feb. 27, 2019, 2:19 p.m. UTC
Commit 5625f2e3266319fd ("TOMOYO: Change pathname for non-rename()able
filesystems.") intended to be applied to filesystems where the content is
not controllable from the userspace (e.g. proc, sysfs, securityfs), based
on an assumption that such filesystems do not support rename() operation.

But it turned out that read-only filesystems also do not support rename()
operation despite the content is controllable from the userspace, and that
commit is annoying TOMOYO users who want to use e.g. squashfs as the root
filesystem due to use of local name which does not start with '/'.

Therefore, based on an assumption that filesystems which require the
device argument upon mount() request is an indication that the content
is controllable from the userspace, do not use local name if a filesystem
does not support rename() operation but requires the device argument upon
mount() request.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 security/tomoyo/realpath.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

James Morris Feb. 27, 2019, 8:25 p.m. UTC | #1
On Wed, 27 Feb 2019, Tetsuo Handa wrote:

> Commit 5625f2e3266319fd ("TOMOYO: Change pathname for non-rename()able
> filesystems.") intended to be applied to filesystems where the content is
> not controllable from the userspace (e.g. proc, sysfs, securityfs), based
> on an assumption that such filesystems do not support rename() operation.
> 
> But it turned out that read-only filesystems also do not support rename()
> operation despite the content is controllable from the userspace, and that
> commit is annoying TOMOYO users who want to use e.g. squashfs as the root
> filesystem due to use of local name which does not start with '/'.
> 
> Therefore, based on an assumption that filesystems which require the
> device argument upon mount() request is an indication that the content
> is controllable from the userspace, do not use local name if a filesystem
> does not support rename() operation but requires the device argument upon
> mount() request.

I'd definitely like Al's input on this.

> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  security/tomoyo/realpath.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
> index 85e6e31..e7832448 100644
> --- a/security/tomoyo/realpath.c
> +++ b/security/tomoyo/realpath.c
> @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path)
>  		 * or dentry without vfsmount.
>  		 */
>  		if (!path->mnt ||
> -		    (!inode->i_op->rename))
> +		    (!inode->i_op->rename &&
> +		     !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
>  			pos = tomoyo_get_local_path(path->dentry, buf,
>  						    buf_len - 1);
>  		/* Get absolute name for the rest. */
>
Tetsuo Handa April 12, 2019, 11:09 a.m. UTC | #2
Al, do you have any concerns with this patch?

On 2019/02/28 5:25, James Morris wrote:
> On Wed, 27 Feb 2019, Tetsuo Handa wrote:
> 
>> Commit 5625f2e3266319fd ("TOMOYO: Change pathname for non-rename()able
>> filesystems.") intended to be applied to filesystems where the content is
>> not controllable from the userspace (e.g. proc, sysfs, securityfs), based
>> on an assumption that such filesystems do not support rename() operation.
>>
>> But it turned out that read-only filesystems also do not support rename()
>> operation despite the content is controllable from the userspace, and that
>> commit is annoying TOMOYO users who want to use e.g. squashfs as the root
>> filesystem due to use of local name which does not start with '/'.
>>
>> Therefore, based on an assumption that filesystems which require the
>> device argument upon mount() request is an indication that the content
>> is controllable from the userspace, do not use local name if a filesystem
>> does not support rename() operation but requires the device argument upon
>> mount() request.
> 
> I'd definitely like Al's input on this.
> 
>>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> ---
>>  security/tomoyo/realpath.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
>> index 85e6e31..e7832448 100644
>> --- a/security/tomoyo/realpath.c
>> +++ b/security/tomoyo/realpath.c
>> @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path)
>>  		 * or dentry without vfsmount.
>>  		 */
>>  		if (!path->mnt ||
>> -		    (!inode->i_op->rename))
>> +		    (!inode->i_op->rename &&
>> +		     !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
>>  			pos = tomoyo_get_local_path(path->dentry, buf,
>>  						    buf_len - 1);
>>  		/* Get absolute name for the rest. */
>>
>
James Morris April 29, 2019, 8:07 p.m. UTC | #3
On Fri, 12 Apr 2019, Tetsuo Handa wrote:

> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >> ---
> >>  security/tomoyo/realpath.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
> >> index 85e6e31..e7832448 100644
> >> --- a/security/tomoyo/realpath.c
> >> +++ b/security/tomoyo/realpath.c
> >> @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path)
> >>  		 * or dentry without vfsmount.
> >>  		 */
> >>  		if (!path->mnt ||
> >> -		    (!inode->i_op->rename))
> >> +		    (!inode->i_op->rename &&
> >> +		     !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
> >>  			pos = tomoyo_get_local_path(path->dentry, buf,
> >>  						    buf_len - 1);
> >>  		/* Get absolute name for the rest. */

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo
diff mbox series

Patch

diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
index 85e6e31..e7832448 100644
--- a/security/tomoyo/realpath.c
+++ b/security/tomoyo/realpath.c
@@ -295,7 +295,8 @@  char *tomoyo_realpath_from_path(const struct path *path)
 		 * or dentry without vfsmount.
 		 */
 		if (!path->mnt ||
-		    (!inode->i_op->rename))
+		    (!inode->i_op->rename &&
+		     !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
 			pos = tomoyo_get_local_path(path->dentry, buf,
 						    buf_len - 1);
 		/* Get absolute name for the rest. */