2016-02-11 15:58:58

by Anton Protopopov

Subject: [PATCH] ext4: ioctl: fix erroneous return value

The ext4_ioctl_setflags() function which is used in the ioctls
EXT4_IOC_SETFLAGS and EXT4_IOC_FSSETXATTR may return the positive value
EPERM instead of -EPERM in case of error. This bug was introduced by a
recent commit 9b7365fc.

The following program can be used to illustrate the wrong behavior:

#include <sys/types.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <err.h>

#define FS_IOC_GETFLAGS _IOR('f', 1, long)
#define FS_IOC_SETFLAGS _IOW('f', 2, long)
#define FS_IMMUTABLE_FL 0x00000010

int main(void)
{
int fd;
long flags;

fd = open("file", O_RDWR|O_CREAT, 0600);
if (fd < 0)
err(1, "open");

if (ioctl(fd, FS_IOC_GETFLAGS, &flags) < 0)
err(1, "ioctl: FS_IOC_GETFLAGS");

flags |= FS_IMMUTABLE_FL;

if (ioctl(fd, FS_IOC_SETFLAGS, &flags) < 0)
err(1, "ioctl: FS_IOC_SETFLAGS");

warnx("ioctl returned no error");

return 0;
}

Running it gives the following result:

$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdbd8bfd38) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdbd8bfd38) = 1
test: ioctl returned no error
+++ exited with 0 +++

Running the program on a kernel with the bug fixed gives the proper result:

$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdd2768258) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdd2768258) = -1 EPERM (Operation not permitted)
test: ioctl: FS_IOC_SETFLAGS: Operation not permitted
+++ exited with 1 +++

Signed-off-by: Anton Protopopov <[email protected]>
---
fs/ext4/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0f6c369..a99b010 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -208,7 +208,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
{
struct ext4_inode_info *ei = EXT4_I(inode);
handle_t *handle = NULL;
- int err = EPERM, migrate = 0;
+ int err = -EPERM, migrate = 0;
struct ext4_iloc iloc;
unsigned int oldflags, mask, i;
unsigned int jflag;
--
2.6.5

2016-02-12 05:02:14

by Theodore Ts'o

Subject: Re: [PATCH] ext4: ioctl: fix erroneous return value

On Thu, Feb 11, 2016 at 10:58:58AM -0500, Anton Protopopov wrote:
> The ext4_ioctl_setflags() function which is used in the ioctls
> EXT4_IOC_SETFLAGS and EXT4_IOC_FSSETXATTR may return the positive value
> EPERM instead of -EPERM in case of error. This bug was introduced by a
> recent commit 9b7365fc.

Nice catch! Thanks, applied.

- Ted