We don't have to do it because it is useless for non regular files.
In fact block device may trigger this path without dentry->d_inode->i_mutex.
Signed-off-by: Dmitri Monakhov <[email protected]>
---
mm/filemap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index a2907d5..3d5a2e7 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1767,7 +1767,7 @@ int should_remove_suid(struct dentry *dentry)
if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
kill |= ATTR_KILL_SGID;
- if (unlikely(kill && !capable(CAP_FSETID)))
+ if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
return kill;
return 0;
--
1.5.4.3
On Sun, 19 Oct 2008 18:00:39 +0400 Dmitri Monakhov <[email protected]> wrote:
> We don't have to do it because it is useless for non regular files.
Are we sure that should_remove_suid() is not and never will used for
directories?
> In fact block device may trigger this path without dentry->d_inode->i_mutex.
What's wrong with blockdevs triggering this path?
Better changelogs, please.
> Signed-off-by: Dmitri Monakhov <[email protected]>
> ---
> mm/filemap.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index a2907d5..3d5a2e7 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1767,7 +1767,7 @@ int should_remove_suid(struct dentry *dentry)
> if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
> kill |= ATTR_KILL_SGID;
>
> - if (unlikely(kill && !capable(CAP_FSETID)))
> + if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
> return kill;
>
> return 0;
Andrew Morton <[email protected]> writes:
> On Sun, 19 Oct 2008 18:00:39 +0400 Dmitri Monakhov <[email protected]> wrote:
>
>> We don't have to do it because it is useless for non regular files.
>
> Are we sure that should_remove_suid() is not and never will used for
> directories?
This bits are meaningless for directory. Posix is quiet about this case.
>
>> In fact block device may trigger this path without dentry->d_inode->i_mutex.
>
> What's wrong with blockdevs triggering this path?
We have following locking rules:notify_changes must be protected
by d_inode->i_mutex.
BTW: You have add this sanity check to notify_change to -mm tree.
>
> Better changelogs, please.
Ok. Will do. My point is: it is improbable what some body(except me :) )
want to set S_ISUID/S_ISGID bit for non regular files. But it is
technically possible and result in unexpected consequence.
>
>> Signed-off-by: Dmitri Monakhov <[email protected]>
>> ---
>> mm/filemap.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index a2907d5..3d5a2e7 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -1767,7 +1767,7 @@ int should_remove_suid(struct dentry *dentry)
>> if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
>> kill |= ATTR_KILL_SGID;
>>
>> - if (unlikely(kill && !capable(CAP_FSETID)))
>> + if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
>> return kill;
>>
>> return 0;
Dmitri Monakhov <[email protected]> writes:
>> Are we sure that should_remove_suid() is not and never will used for
>> directories?
> This bits are meaningless for directory. Posix is quiet about this case.
> Ok. Will do. My point is: it is improbable what some body(except me :) )
> want to set S_ISUID/S_ISGID bit for non regular files. But it is
> technically possible and result in unexpected consequence.
I don't know about the context, but SGID bits for directories are not
magic, they mean the newly created files will inherit GID from the
parent directory, and if the file is actually a subdirectory, it will
also have SGID set. The same was also true WRT SUID bits on some
systems.
--
Krzysztof Halasa