2017-04-27 16:34:10

by Alexander Graf

[permalink] [raw]
Subject: [PATCH v2] ceph: Fix file open flags on ppc64

The file open flags (O_foo) are platform specific and should never go
out to an interface that is not local to the system.

Unfortunately these flags have leaked out onto the wire in the cephfs
implementation. That lead to bogus flags getting transmitted on ppc64.

This patch converts the kernel view of flags to the ceph view of file
open flags.

Fixes: 124e68e74 ("ceph: file operations")
Signed-off-by: Alexander Graf <[email protected]>

---

v1 -> v2:

- Only convert flags mds knows about
- Fix le conversion
---
fs/ceph/file.c | 35 ++++++++++++++++++++++++++++++++++-
include/linux/ceph/ceph_fs.h | 12 ++++++++++++
2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 26cc954..9cac018 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -103,6 +103,39 @@ static size_t dio_get_pagev_size(const struct iov_iter *it)
return ERR_PTR(ret);
}

+static __le32 ceph_flags_sys2wire(u32 flags)
+{
+ u32 wire_flags = 0;
+
+ switch (flags & O_ACCMODE) {
+ case O_RDONLY:
+ wire_flags |= CEPH_O_RDONLY;
+ break;
+ case O_WRONLY:
+ wire_flags |= CEPH_O_WRONLY;
+ break;
+ case O_RDWR:
+ wire_flags |= CEPH_O_RDWR;
+ break;
+ }
+ flags &= ~O_ACCMODE;
+
+#define ceph_sys2wire(a) if (flags & a) { wire_flags |= CEPH_##a; flags &= ~a; }
+
+ ceph_sys2wire(O_CREAT);
+ ceph_sys2wire(O_EXCL);
+ ceph_sys2wire(O_TRUNC);
+ ceph_sys2wire(O_DIRECTORY);
+ ceph_sys2wire(O_NOFOLLOW);
+
+#undef ceph_sys2wire
+
+ if (flags)
+ dout("Unused open flags: %x", flags);
+
+ return cpu_to_le32(wire_flags);
+}
+
/*
* Prepare an open request. Preallocate ceph_cap to avoid an
* inopportune ENOMEM later.
@@ -123,7 +156,7 @@ static size_t dio_get_pagev_size(const struct iov_iter *it)
if (IS_ERR(req))
goto out;
req->r_fmode = ceph_flags_to_mode(flags);
- req->r_args.open.flags = cpu_to_le32(flags);
+ req->r_args.open.flags = ceph_flags_sys2wire(flags);
req->r_args.open.mode = cpu_to_le32(create_mode);
out:
return req;
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index f4b2ee1..9581c70 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -366,6 +366,18 @@ enum {
#define CEPH_READDIR_FRAG_COMPLETE (1<<8)
#define CEPH_READDIR_HASH_ORDER (1<<9)

+/*
+ * open request flags
+ */
+#define CEPH_O_RDONLY 00000000
+#define CEPH_O_WRONLY 00000001
+#define CEPH_O_RDWR 00000002
+#define CEPH_O_CREAT 00000100
+#define CEPH_O_EXCL 00000200
+#define CEPH_O_TRUNC 00001000
+#define CEPH_O_DIRECTORY 00200000
+#define CEPH_O_NOFOLLOW 00400000
+
union ceph_mds_request_args {
struct {
__le32 mask; /* CEPH_CAP_* */
--
1.8.5.6


2017-04-28 07:58:05

by Yan, Zheng

[permalink] [raw]
Subject: Re: [PATCH v2] ceph: Fix file open flags on ppc64


> On 28 Apr 2017, at 00:34, Alexander Graf <[email protected]> wrote:
>
> The file open flags (O_foo) are platform specific and should never go
> out to an interface that is not local to the system.
>
> Unfortunately these flags have leaked out onto the wire in the cephfs
> implementation. That lead to bogus flags getting transmitted on ppc64.
>
> This patch converts the kernel view of flags to the ceph view of file
> open flags.
>
> Fixes: 124e68e74 ("ceph: file operations")
> Signed-off-by: Alexander Graf <[email protected]>
>
> —

I removed the "unused open flags” dout and applied the patch. Thank you for tracking down and fixing the bug.

Regards
Yan, Zheng

>
> v1 -> v2:
>
> - Only convert flags mds knows about
> - Fix le conversion
> ---
> fs/ceph/file.c | 35 ++++++++++++++++++++++++++++++++++-
> include/linux/ceph/ceph_fs.h | 12 ++++++++++++
> 2 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 26cc954..9cac018 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -103,6 +103,39 @@ static size_t dio_get_pagev_size(const struct iov_iter *it)
> return ERR_PTR(ret);
> }
>
> +static __le32 ceph_flags_sys2wire(u32 flags)
> +{
> + u32 wire_flags = 0;
> +
> + switch (flags & O_ACCMODE) {
> + case O_RDONLY:
> + wire_flags |= CEPH_O_RDONLY;
> + break;
> + case O_WRONLY:
> + wire_flags |= CEPH_O_WRONLY;
> + break;
> + case O_RDWR:
> + wire_flags |= CEPH_O_RDWR;
> + break;
> + }
> + flags &= ~O_ACCMODE;
> +
> +#define ceph_sys2wire(a) if (flags & a) { wire_flags |= CEPH_##a; flags &= ~a; }
> +
> + ceph_sys2wire(O_CREAT);
> + ceph_sys2wire(O_EXCL);
> + ceph_sys2wire(O_TRUNC);
> + ceph_sys2wire(O_DIRECTORY);
> + ceph_sys2wire(O_NOFOLLOW);
> +
> +#undef ceph_sys2wire
> +
> + if (flags)
> + dout("Unused open flags: %x", flags);
> +
> + return cpu_to_le32(wire_flags);
> +}
> +
> /*
> * Prepare an open request. Preallocate ceph_cap to avoid an
> * inopportune ENOMEM later.
> @@ -123,7 +156,7 @@ static size_t dio_get_pagev_size(const struct iov_iter *it)
> if (IS_ERR(req))
> goto out;
> req->r_fmode = ceph_flags_to_mode(flags);
> - req->r_args.open.flags = cpu_to_le32(flags);
> + req->r_args.open.flags = ceph_flags_sys2wire(flags);
> req->r_args.open.mode = cpu_to_le32(create_mode);
> out:
> return req;
> diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
> index f4b2ee1..9581c70 100644
> --- a/include/linux/ceph/ceph_fs.h
> +++ b/include/linux/ceph/ceph_fs.h
> @@ -366,6 +366,18 @@ enum {
> #define CEPH_READDIR_FRAG_COMPLETE (1<<8)
> #define CEPH_READDIR_HASH_ORDER (1<<9)
>
> +/*
> + * open request flags
> + */
> +#define CEPH_O_RDONLY 00000000
> +#define CEPH_O_WRONLY 00000001
> +#define CEPH_O_RDWR 00000002
> +#define CEPH_O_CREAT 00000100
> +#define CEPH_O_EXCL 00000200
> +#define CEPH_O_TRUNC 00001000
> +#define CEPH_O_DIRECTORY 00200000
> +#define CEPH_O_NOFOLLOW 00400000
> +
> union ceph_mds_request_args {
> struct {
> __le32 mask; /* CEPH_CAP_* */
> --
> 1.8.5.6
>

2017-04-28 09:17:08

by Alexander Graf

[permalink] [raw]
Subject: Re: [PATCH v2] ceph: Fix file open flags on ppc64



On 28.04.17 09:57, Yan, Zheng wrote:
>
>> On 28 Apr 2017, at 00:34, Alexander Graf <[email protected]> wrote:
>>
>> The file open flags (O_foo) are platform specific and should never go
>> out to an interface that is not local to the system.
>>
>> Unfortunately these flags have leaked out onto the wire in the cephfs
>> implementation. That lead to bogus flags getting transmitted on ppc64.
>>
>> This patch converts the kernel view of flags to the ceph view of file
>> open flags.
>>
>> Fixes: 124e68e74 ("ceph: file operations")
>> Signed-off-by: Alexander Graf <[email protected]>
>>
>> —
>
> I removed the "unused open flags” dout and applied the patch. Thank you for tracking down and fixing the bug.

I actually put that in on purpose, in case anyone in 2 years tries to
find out why a particular flag doesn't get populated :).


Alex

2017-04-28 09:24:05

by Yan, Zheng

[permalink] [raw]
Subject: Re: [PATCH v2] ceph: Fix file open flags on ppc64


> On 28 Apr 2017, at 17:14, Alexander Graf <[email protected]> wrote:
>
>
>
> On 28.04.17 09:57, Yan, Zheng wrote:
>>
>>> On 28 Apr 2017, at 00:34, Alexander Graf <[email protected]> wrote:
>>>
>>> The file open flags (O_foo) are platform specific and should never go
>>> out to an interface that is not local to the system.
>>>
>>> Unfortunately these flags have leaked out onto the wire in the cephfs
>>> implementation. That lead to bogus flags getting transmitted on ppc64.
>>>
>>> This patch converts the kernel view of flags to the ceph view of file
>>> open flags.
>>>
>>> Fixes: 124e68e74 ("ceph: file operations")
>>> Signed-off-by: Alexander Graf <[email protected]>
>>>
>>> —
>>
>> I removed the "unused open flags” dout and applied the patch. Thank you for tracking down and fixing the bug.
>
> I actually put that in on purpose, in case anyone in 2 years tries to find out why a particular flag doesn't get populated :).
>
Ok, I will put it back.

Regards
Yan, Zheng

>
> Alex