| 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
| ||
|
Message-ID: <1498592170.4830.8.camel@poochiereds.net>
Date: Tue, 27 Jun 2017 15:36:10 -0400
From: Jeff Layton <jlayton@...chiereds.net>
To: Benjamin Coddington <bcodding@...hat.com>,
Oleg Drokin <oleg.drokin@...el.com>,
Andreas Dilger <andreas.dilger@...el.com>,
James Simmons <jsimmons@...radead.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Eric Van Hensbergen <ericvh@...il.com>,
Ron Minnich <rminnich@...dia.gov>,
Latchesar Ionkov <lucho@...kov.net>,
"Yan, Zheng" <zyan@...hat.com>, Sage Weil <sage@...hat.com>,
Ilya Dryomov <idryomov@...il.com>,
Steve French <sfrench@...ba.org>,
Christine Caulfield <ccaulfie@...hat.com>,
David Teigland <teigland@...hat.com>,
Miklos Szeredi <miklos@...redi.hu>,
Alexander Viro <viro@...iv.linux.org.uk>,
"J. Bruce Fields" <bfields@...ldses.org>,
Vitaly Fertman <vitaly_fertman@...atex.com>,
"John L. Hammond" <john.hammond@...el.com>,
Andriy Skulysh <andriy.skulysh@...gate.com>,
Emoly Liu <emoly.liu@...el.com>
Cc: lustre-devel@...ts.lustre.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org, v9fs-developer@...ts.sourceforge.net,
ceph-devel@...r.kernel.org, linux-cifs@...r.kernel.org,
samba-technical@...ts.samba.org, cluster-devel@...hat.com,
linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 3/3] staging/lustre, 9p, ceph, cifs, dlm: negate remote
pids for F_GETLK
On Tue, 2017-06-27 at 11:18 -0400, Benjamin Coddington wrote:
> In the previous patch, the locks API will expect that if a filesystem
> returns a remote pid as opposed to a local pid for F_GETLK, that remote pid
> will be <= 0. This signifies that the pid is remote, and the locks API
> will forego translating that pid into the pid namespace of the local
> calling process. Since local pids will never be larger than PID_MAX_LIMIT
> (which is currently defined as <= 4 million), but pid_t is an unsigned int,
> we should have plenty of room to represent remote pids with negative
> numbers if we assume that remote pid numbers are similarly limited. If
> this is not the case, then we run the risk of having a remote pid returned
> for which there is also a corresponding local pid. This is a problem we
> have now, but this patch should reduce the chances of that occurring, while
> also returning those remote pid numbers, for whatever that may be worth.
>
> This patch updates lustre, 9p, ceph, cifs, and dlm to negate the remote pid
> returned for F_GETLK lock requests.
>
> Signed-off-by: Benjamin Coddington <bcodding@...hat.com>
> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_flock.c | 2 +-
> fs/9p/vfs_file.c | 2 +-
> fs/ceph/locks.c | 2 +-
> fs/cifs/cifssmb.c | 2 +-
> fs/dlm/plock.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> index b7f28b39c7b3..abcbf075acc0 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
> @@ -596,7 +596,7 @@ ldlm_flock_completion_ast(struct ldlm_lock *lock, __u64 flags, void *data)
> default:
> getlk->fl_type = F_UNLCK;
> }
> - getlk->fl_pid = (pid_t)lock->l_policy_data.l_flock.pid;
> + getlk->fl_pid = -(pid_t)lock->l_policy_data.l_flock.pid;
> getlk->fl_start = (loff_t)lock->l_policy_data.l_flock.start;
> getlk->fl_end = (loff_t)lock->l_policy_data.l_flock.end;
> } else {
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 3de3b4a89d89..43c242e17132 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -288,7 +288,7 @@ static int v9fs_file_getlock(struct file *filp, struct file_lock *fl)
> fl->fl_end = OFFSET_MAX;
> else
> fl->fl_end = glock.start + glock.length - 1;
> - fl->fl_pid = glock.proc_id;
> + fl->fl_pid = -glock.proc_id;
> }
> kfree(glock.client_id);
> return res;
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index 6806dbeaee19..0fd5c288ce4e 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -79,7 +79,7 @@ static int ceph_lock_message(u8 lock_type, u16 operation, struct file *file,
> err = ceph_mdsc_do_request(mdsc, inode, req);
>
> if (operation == CEPH_MDS_OP_GETFILELOCK) {
> - fl->fl_pid = le64_to_cpu(req->r_reply_info.filelock_reply->pid);
> + fl->fl_pid = -le64_to_cpu(req->r_reply_info.filelock_reply->pid);
> if (CEPH_LOCK_SHARED == req->r_reply_info.filelock_reply->type)
> fl->fl_type = F_RDLCK;
> else if (CEPH_LOCK_EXCL == req->r_reply_info.filelock_reply->type)
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index fbb0d4cbda41..cb367050f972 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -2515,7 +2515,7 @@ CIFSSMBPosixLock(const unsigned int xid, struct cifs_tcon *tcon,
> pLockData->fl_start = le64_to_cpu(parm_data->start);
> pLockData->fl_end = pLockData->fl_start +
> le64_to_cpu(parm_data->length) - 1;
> - pLockData->fl_pid = le32_to_cpu(parm_data->pid);
> + pLockData->fl_pid = -le32_to_cpu(parm_data->pid);
> }
> }
>
> diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
> index d401425f602a..e631b1689228 100644
> --- a/fs/dlm/plock.c
> +++ b/fs/dlm/plock.c
> @@ -367,7 +367,7 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number, struct file *file,
> locks_init_lock(fl);
> fl->fl_type = (op->info.ex) ? F_WRLCK : F_RDLCK;
> fl->fl_flags = FL_POSIX;
> - fl->fl_pid = op->info.pid;
> + fl->fl_pid = -op->info.pid;
> fl->fl_start = op->info.start;
> fl->fl_end = op->info.end;
> rv = 0;
I think this is probably a reasonable thing to do, given that we also
report OFD locks today with an l_pid of -1. The pid on any sort of
distributed fs is pretty meaningless anyway.
I think this all looks good. I'll plan to merge it for -next in a bit
and do some testing with it.
Thanks!
--
Jeff Layton <jlayton@...chiereds.net>
Powered by blists - more mailing lists