* [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error
@ 2011-10-28 18:03 Eryu Guan
2011-11-01 23:10 ` Jan Kara
0 siblings, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2011-10-28 18:03 UTC (permalink / raw)
To: linux-ext4; +Cc: Eryu Guan, Jan Kara
Newly created file on ext3 inherits inode flags from parent directory,
so new inode created in append-only directory has S_APPEND flag set,
may_open() called by do_last() checks that flag then returns -EPERM,
but at that time the new inode is already created.
This can be reproduced by:
# mkdir -p /mnt/ext3/append-only
# chattr +a /mnt/ext3/append-only
# ./opentest /mnt/ext3/append-only/newtestfile
# ls -l /mnt/ext3/append-only/newtestfile
opentest will return 'Operation not permitted', but the ls shows that
newtestfile is already created.
# cat opentest.c
#include <stdio.h>
#include <sys/types.h>
#include <fcntl.h>
#include <sys/stat.h>
int main(int argc, char *argv[])
{
int fd;
fd = open(argv[1], O_RDWR|O_CREAT, 0666);
if (fd == -1)
perror("open failed");
return 0;
}
To avoid this, check EXT3_APPEND_FL flag first in ext3_create before
really allocating new inode.
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Eryu Guan <guaneryu@gmail.com>
---
fs/ext3/namei.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index 0629e09..323cf2f 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -36,6 +36,7 @@
#include <linux/quotaops.h>
#include <linux/buffer_head.h>
#include <linux/bio.h>
+#include <linux/namei.h>
#include <trace/events/ext3.h>
#include "namei.h"
@@ -1704,6 +1705,15 @@ static int ext3_create (struct inode * dir, struct dentry * dentry, int mode,
handle_t *handle;
struct inode * inode;
int err, retries = 0;
+ int open_flag = nd->intent.open.file->f_flags;
+
+ if ((EXT3_I(dir)->i_flags & EXT3_FL_INHERITED) & EXT3_APPEND_FL) {
+ if ((open_flag & O_ACCMODE) != O_RDONLY &&
+ !(open_flag & O_APPEND))
+ return -EPERM;
+ if (open_flag & O_TRUNC)
+ return -EPERM;
+ }
dquot_initialize(dir);
--
1.7.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error
2011-10-28 18:03 [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error Eryu Guan
@ 2011-11-01 23:10 ` Jan Kara
2011-11-01 23:27 ` Ted Ts'o
2011-11-02 2:05 ` Eryu Guan
0 siblings, 2 replies; 5+ messages in thread
From: Jan Kara @ 2011-11-01 23:10 UTC (permalink / raw)
To: Eryu Guan; +Cc: linux-ext4, Jan Kara
On Sat 29-10-11 02:03:07, Eryu Guan wrote:
> Newly created file on ext3 inherits inode flags from parent directory,
> so new inode created in append-only directory has S_APPEND flag set,
> may_open() called by do_last() checks that flag then returns -EPERM,
> but at that time the new inode is already created.
>
> This can be reproduced by:
> # mkdir -p /mnt/ext3/append-only
> # chattr +a /mnt/ext3/append-only
> # ./opentest /mnt/ext3/append-only/newtestfile
> # ls -l /mnt/ext3/append-only/newtestfile
>
> opentest will return 'Operation not permitted', but the ls shows that
> newtestfile is already created.
>
> # cat opentest.c
> #include <stdio.h>
> #include <sys/types.h>
> #include <fcntl.h>
> #include <sys/stat.h>
>
> int main(int argc, char *argv[])
> {
> int fd;
> fd = open(argv[1], O_RDWR|O_CREAT, 0666);
> if (fd == -1)
> perror("open failed");
> return 0;
> }
>
> To avoid this, check EXT3_APPEND_FL flag first in ext3_create before
> really allocating new inode.
Yes, it is nicer to not create any file when open(2) fails in the end.
BTW, how have you spotted this? I've taken your ext2 and ext3 patches into
my tree.
Honza
>
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Eryu Guan <guaneryu@gmail.com>
> ---
> fs/ext3/namei.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index 0629e09..323cf2f 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -36,6 +36,7 @@
> #include <linux/quotaops.h>
> #include <linux/buffer_head.h>
> #include <linux/bio.h>
> +#include <linux/namei.h>
> #include <trace/events/ext3.h>
>
> #include "namei.h"
> @@ -1704,6 +1705,15 @@ static int ext3_create (struct inode * dir, struct dentry * dentry, int mode,
> handle_t *handle;
> struct inode * inode;
> int err, retries = 0;
> + int open_flag = nd->intent.open.file->f_flags;
> +
> + if ((EXT3_I(dir)->i_flags & EXT3_FL_INHERITED) & EXT3_APPEND_FL) {
> + if ((open_flag & O_ACCMODE) != O_RDONLY &&
> + !(open_flag & O_APPEND))
> + return -EPERM;
> + if (open_flag & O_TRUNC)
> + return -EPERM;
> + }
>
> dquot_initialize(dir);
>
> --
> 1.7.7.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error
2011-11-01 23:10 ` Jan Kara
@ 2011-11-01 23:27 ` Ted Ts'o
2011-11-02 14:23 ` Jan Kara
2011-11-02 2:05 ` Eryu Guan
1 sibling, 1 reply; 5+ messages in thread
From: Ted Ts'o @ 2011-11-01 23:27 UTC (permalink / raw)
To: Jan Kara; +Cc: Eryu Guan, linux-ext4
On Wed, Nov 02, 2011 at 12:10:34AM +0100, Jan Kara wrote:
> >
> > To avoid this, check EXT3_APPEND_FL flag first in ext3_create before
> > really allocating new inode.
> Yes, it is nicer to not create any file when open(2) fails in the end.
> BTW, how have you spotted this? I've taken your ext2 and ext3 patches into
> my tree.
Note: I have a fix in my tree which removes EXTx_APPEND_FL from the
set of flags that can be inherited from the containing directory in
ext2, ext3, and ext4. That addresses this issue without needing to
make the change in this patch.
- Ted
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error
2011-11-01 23:27 ` Ted Ts'o
@ 2011-11-02 14:23 ` Jan Kara
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2011-11-02 14:23 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Jan Kara, Eryu Guan, linux-ext4
On Tue 01-11-11 19:27:25, Ted Tso wrote:
> On Wed, Nov 02, 2011 at 12:10:34AM +0100, Jan Kara wrote:
> > >
> > > To avoid this, check EXT3_APPEND_FL flag first in ext3_create before
> > > really allocating new inode.
> > Yes, it is nicer to not create any file when open(2) fails in the end.
> > BTW, how have you spotted this? I've taken your ext2 and ext3 patches into
> > my tree.
>
> Note: I have a fix in my tree which removes EXTx_APPEND_FL from the
> set of flags that can be inherited from the containing directory in
> ext2, ext3, and ext4. That addresses this issue without needing to
> make the change in this patch.
Ah, OK. I forgot about it. I've removed the two patches from my queue.
Thanks for letting me know.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error
2011-11-01 23:10 ` Jan Kara
2011-11-01 23:27 ` Ted Ts'o
@ 2011-11-02 2:05 ` Eryu Guan
1 sibling, 0 replies; 5+ messages in thread
From: Eryu Guan @ 2011-11-02 2:05 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-ext4
On Wed, Nov 2, 2011 at 7:10 AM, Jan Kara <jack@suse.cz> wrote:
> On Sat 29-10-11 02:03:07, Eryu Guan wrote:
>> Newly created file on ext3 inherits inode flags from parent directory,
>> so new inode created in append-only directory has S_APPEND flag set,
>> may_open() called by do_last() checks that flag then returns -EPERM,
>> but at that time the new inode is already created.
>>
>> This can be reproduced by:
>> # mkdir -p /mnt/ext3/append-only
>> # chattr +a /mnt/ext3/append-only
>> # ./opentest /mnt/ext3/append-only/newtestfile
>> # ls -l /mnt/ext3/append-only/newtestfile
>>
>> opentest will return 'Operation not permitted', but the ls shows that
>> newtestfile is already created.
>>
>> # cat opentest.c
>> #include <stdio.h>
>> #include <sys/types.h>
>> #include <fcntl.h>
>> #include <sys/stat.h>
>>
>> int main(int argc, char *argv[])
>> {
>> int fd;
>> fd = open(argv[1], O_RDWR|O_CREAT, 0666);
>> if (fd == -1)
>> perror("open failed");
>> return 0;
>> }
>>
>> To avoid this, check EXT3_APPEND_FL flag first in ext3_create before
>> really allocating new inode.
> Yes, it is nicer to not create any file when open(2) fails in the end.
> BTW, how have you spotted this? I've taken your ext2 and ext3 patches into
> my tree.
I found this by xfstests 079, since it's no longer xfs specific and
fails on extN.
After applying my patch, 079 still fails on extN, but not leave
unwanted file there.
As Ted said, the other way to fix this is to remove EXTx_APPEND_FL from
EXTx_FL_INHERITED, so new inode will not inherit the append-only flag. This fix
will also make 079 pass on extN.
Though I prefer the later way, I still sent out my patch for review,
because it's
not a behavior-changer.
Thanks.
Eryu Guan
>
> Honza
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Signed-off-by: Eryu Guan <guaneryu@gmail.com>
>> ---
>> fs/ext3/namei.c | 10 ++++++++++
>> 1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
>> index 0629e09..323cf2f 100644
>> --- a/fs/ext3/namei.c
>> +++ b/fs/ext3/namei.c
>> @@ -36,6 +36,7 @@
>> #include <linux/quotaops.h>
>> #include <linux/buffer_head.h>
>> #include <linux/bio.h>
>> +#include <linux/namei.h>
>> #include <trace/events/ext3.h>
>>
>> #include "namei.h"
>> @@ -1704,6 +1705,15 @@ static int ext3_create (struct inode * dir, struct dentry * dentry, int mode,
>> handle_t *handle;
>> struct inode * inode;
>> int err, retries = 0;
>> + int open_flag = nd->intent.open.file->f_flags;
>> +
>> + if ((EXT3_I(dir)->i_flags & EXT3_FL_INHERITED) & EXT3_APPEND_FL) {
>> + if ((open_flag & O_ACCMODE) != O_RDONLY &&
>> + !(open_flag & O_APPEND))
>> + return -EPERM;
>> + if (open_flag & O_TRUNC)
>> + return -EPERM;
>> + }
>>
>> dquot_initialize(dir);
>>
>> --
>> 1.7.7.1
>>
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-02 14:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-28 18:03 [PATCH] ext3: Avoid creating new file in append-only dir when open(2) return error Eryu Guan
2011-11-01 23:10 ` Jan Kara
2011-11-01 23:27 ` Ted Ts'o
2011-11-02 14:23 ` Jan Kara
2011-11-02 2:05 ` Eryu Guan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).