[v2,2/4] ceph: use pathlen values returned by set_request_path_attr
diff mbox series

Message ID 20190417183959.11296-3-jlayton@kernel.org
State New
Headers show
Series
  • ceph: dentry->d_name handing fixes
Related show

Commit Message

Jeff Layton April 17, 2019, 6:39 p.m. UTC
We make copies of the dentry name in set_request_path_attr, but then
create_request_message re-fetches the lengths out of the dentry. Those
lengths may not be correct if we race with a rename. Use the pathlen
values that set_request_path_attr returned instead.

Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Yan, Zheng April 18, 2019, 2:33 a.m. UTC | #1
On Thu, Apr 18, 2019 at 2:43 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> We make copies of the dentry name in set_request_path_attr, but then
> create_request_message re-fetches the lengths out of the dentry. Those
> lengths may not be correct if we race with a rename. Use the pathlen
> values that set_request_path_attr returned instead.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/mds_client.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 4cfefe118128..f362c16c5119 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2305,9 +2305,9 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
>                 (!!req->r_inode_drop + !!req->r_dentry_drop +
>                  !!req->r_old_inode_drop + !!req->r_old_dentry_drop);
>         if (req->r_dentry_drop)
> -               len += req->r_dentry->d_name.len;
> +               len += pathlen1;
>         if (req->r_old_dentry_drop)
> -               len += req->r_old_dentry->d_name.len;
> +               len += pathlen2;
>
>         msg = ceph_msg_new2(CEPH_MSG_CLIENT_REQUEST, len, 1, GFP_NOFS, false);
>         if (!msg) {
> --
> 2.20.1
>

This patch may not be needed. because when r_dentry_drop or
r_old_dentry_drop is set, parent dir should be locked.
Jeff Layton April 18, 2019, 11:18 a.m. UTC | #2
On Wed, Apr 17, 2019 at 10:33 PM Yan, Zheng <ukernel@gmail.com> wrote:
>
> On Thu, Apr 18, 2019 at 2:43 AM Jeff Layton <jlayton@kernel.org> wrote:
> >
> > We make copies of the dentry name in set_request_path_attr, but then
> > create_request_message re-fetches the lengths out of the dentry. Those
> > lengths may not be correct if we race with a rename. Use the pathlen
> > values that set_request_path_attr returned instead.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/mds_client.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index 4cfefe118128..f362c16c5119 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -2305,9 +2305,9 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> >                 (!!req->r_inode_drop + !!req->r_dentry_drop +
> >                  !!req->r_old_inode_drop + !!req->r_old_dentry_drop);
> >         if (req->r_dentry_drop)
> > -               len += req->r_dentry->d_name.len;
> > +               len += pathlen1;
> >         if (req->r_old_dentry_drop)
> > -               len += req->r_old_dentry->d_name.len;
> > +               len += pathlen2;
> >
> >         msg = ceph_msg_new2(CEPH_MSG_CLIENT_REQUEST, len, 1, GFP_NOFS, false);
> >         if (!msg) {
> > --
> > 2.20.1
> >
>
> This patch may not be needed. because when r_dentry_drop or
> r_old_dentry_drop is set, parent dir should be locked.

Got it. Ok, let's drop this one for v5.1 and stable.

That said, I'd prefer we not rely on those sorts of subtle
assumptions. How about we take this one for v5.2?
Yan, Zheng April 19, 2019, 1:28 a.m. UTC | #3
On Thu, Apr 18, 2019 at 7:18 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Wed, Apr 17, 2019 at 10:33 PM Yan, Zheng <ukernel@gmail.com> wrote:
> >
> > On Thu, Apr 18, 2019 at 2:43 AM Jeff Layton <jlayton@kernel.org> wrote:
> > >
> > > We make copies of the dentry name in set_request_path_attr, but then
> > > create_request_message re-fetches the lengths out of the dentry. Those
> > > lengths may not be correct if we race with a rename. Use the pathlen
> > > values that set_request_path_attr returned instead.
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/ceph/mds_client.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > index 4cfefe118128..f362c16c5119 100644
> > > --- a/fs/ceph/mds_client.c
> > > +++ b/fs/ceph/mds_client.c
> > > @@ -2305,9 +2305,9 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> > >                 (!!req->r_inode_drop + !!req->r_dentry_drop +
> > >                  !!req->r_old_inode_drop + !!req->r_old_dentry_drop);
> > >         if (req->r_dentry_drop)
> > > -               len += req->r_dentry->d_name.len;
> > > +               len += pathlen1;
> > >         if (req->r_old_dentry_drop)
> > > -               len += req->r_old_dentry->d_name.len;
> > > +               len += pathlen2;
> > >
> > >         msg = ceph_msg_new2(CEPH_MSG_CLIENT_REQUEST, len, 1, GFP_NOFS, false);
> > >         if (!msg) {
> > > --
> > > 2.20.1
> > >
> >
> > This patch may not be needed. because when r_dentry_drop or
> > r_old_dentry_drop is set, parent dir should be locked.
>
> Got it. Ok, let's drop this one for v5.1 and stable.
>
> That said, I'd prefer we not rely on those sorts of subtle
> assumptions. How about we take this one for v5.2?

But ceph_encode_dentry_release() still uses r_dentry directly.  It
needs to be updated too.


> --
> Jeff Layton <jlayton@kernel.org>

Patch
diff mbox series

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 4cfefe118128..f362c16c5119 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2305,9 +2305,9 @@  static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
 		(!!req->r_inode_drop + !!req->r_dentry_drop +
 		 !!req->r_old_inode_drop + !!req->r_old_dentry_drop);
 	if (req->r_dentry_drop)
-		len += req->r_dentry->d_name.len;
+		len += pathlen1;
 	if (req->r_old_dentry_drop)
-		len += req->r_old_dentry->d_name.len;
+		len += pathlen2;
 
 	msg = ceph_msg_new2(CEPH_MSG_CLIENT_REQUEST, len, 1, GFP_NOFS, false);
 	if (!msg) {