linux-kernel - Re: [PATCH] ceph: Fix file open flags on ppc64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 21 Apr 2017 08:59:39 +0200
From:   Jan Fajerski <jfajerski@...e.com>
To:     "Yan, Zheng" <zyan@...hat.com>
Cc:     Alexander Graf <agraf@...e.de>,
        ceph-devel <ceph-devel@...r.kernel.org>,
        Sage Weil <sage@...hat.com>, Ilya Dryomov <idryomov@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jan.Fajerski@...e.com
Subject: Re: [PATCH] ceph: Fix file open flags on ppc64

On Fri, Apr 21, 2017 at 10:22:16AM +0800, Yan, Zheng wrote:
>
>> On 20 Apr 2017, at 20:40, Alexander Graf <agraf@...e.de> 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. On x86 systems, the new function should get optimized out
>> by smart compilers. On systems where the flags differ, it should adapt
>> them accordingly.
>>
>> Fixes: 124e68e74 ("ceph: file operations")
>> Signed-off-by: Alexander Graf <agraf@...e.de>
>> ---
>> fs/ceph/file.c               | 45 +++++++++++++++++++++++++++++++++++++++++++-
>> include/linux/ceph/ceph_fs.h | 24 +++++++++++++++++++++++
>> 2 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 26cc954..0ed6392 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -103,6 +103,49 @@ static size_t dio_get_pagev_size(const struct iov_iter *it)
>> 	return ERR_PTR(ret);
>> }
>>
>> +static u32 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_NOCTTY);
>> +	ceph_sys2wire(O_TRUNC);
>> +	ceph_sys2wire(O_APPEND);
>> +	ceph_sys2wire(O_NONBLOCK);
>> +	ceph_sys2wire(O_DSYNC);
>> +	ceph_sys2wire(FASYNC);
>> +	ceph_sys2wire(O_DIRECT);
>> +	ceph_sys2wire(O_LARGEFILE);
>> +	ceph_sys2wire(O_DIRECTORY);
>> +	ceph_sys2wire(O_NOFOLLOW);
>> +	ceph_sys2wire(O_NOATIME);
>> +	ceph_sys2wire(O_CLOEXEC);
>> +	ceph_sys2wire(__O_SYNC);
>> +	ceph_sys2wire(O_PATH);
>> +	ceph_sys2wire(__O_TMPFILE);
>> +
>> +#undef ceph_sys2wire
>> +
>> +	WARN_ONCE(flags, "Found unknown open flags: %x", flags);
>> +
>> +	return wire_flags;
>> +}
>> +
>> /*
>>  * Prepare an open request.  Preallocate ceph_cap to avoid an
>>  * inopportune ENOMEM later.
>> @@ -123,7 +166,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(cpu_to_le32(flags));
>
>cpu_to_le32(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..d254b48 100644
>> --- a/include/linux/ceph/ceph_fs.h
>> +++ b/include/linux/ceph/ceph_fs.h
>> @@ -366,6 +366,30 @@ 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_NOCTTY		00000400
>> +#define CEPH_O_TRUNC		00001000
>> +#define CEPH_O_APPEND		00002000
>> +#define CEPH_O_NONBLOCK		00004000
>> +#define CEPH_O_DSYNC		00010000
>> +#define CEPH_FASYNC		00020000
>> +#define CEPH_O_DIRECT		00040000
>> +#define CEPH_O_LARGEFILE	00100000
>> +#define CEPH_O_DIRECTORY	00200000
>> +#define CEPH_O_NOFOLLOW		00400000
>> +#define CEPH_O_NOATIME		01000000
>> +#define CEPH_O_CLOEXEC		02000000
>> +#define CEPH___O_SYNC		04000000
>> +#define CEPH_O_PATH		010000000
>> +#define CEPH___O_TMPFILE	020000000
>> +
>
>RDONLY, WRONLY, RDWR, CREAT, EXCL, TRUNC, DIRECTORY, NOFOLLOW  are used by mds, no need to define the rest.
>
>> union ceph_mds_request_args {
>> 	struct {
>> 		__le32 mask;                 /* CEPH_CAP_* */
>
>Besides, the ceph mds server requires the fix too. Do you have patch for that?
If the wire format is normalized like this, the mds can remain unchanged, can it 
not?
>
>Regards
>Yan, Zheng
>
>> --
>> 1.8.5.6
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>the body of a message to majordomo@...r.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Jan Fajerski
Engineer Enterprise Storage
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)