Patchwork cifs: fix dentry hash calculation for case-insensitive mounts

login
register
mail settings
Submitter Jeff Layton
Date Feb. 5, 2010, 6:30 p.m.
Message ID <1265394636-7508-1-git-send-email-jlayton@redhat.com>
Download mbox | patch
Permalink /patch/44657/
State New
Headers show

Comments

Jeff Layton - Feb. 5, 2010, 6:30 p.m.
case-insensitive mounts shouldn't use full_name_hash(). Make sure we
use the parent dentry's d_hash routine when one is set.

Reported-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/readdir.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Shirish Pargaonkar - Feb. 5, 2010, 8:31 p.m.
On Fri, Feb 5, 2010 at 12:30 PM, Jeff Layton <jlayton@redhat.com> wrote:
> case-insensitive mounts shouldn't use full_name_hash(). Make sure we
> use the parent dentry's d_hash routine when one is set.
>
> Reported-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/readdir.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> index f5618f8..c343b14 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -77,6 +77,11 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
>
>        cFYI(1, ("For %s", name->name));
>
> +       if (parent->d_op && parent->d_op->d_hash)
> +               parent->d_op->d_hash(parent, name);
> +       else
> +               name->hash = full_name_hash(name->name, name->len);
> +
>        dentry = d_lookup(parent, name);
>        if (dentry) {
>                /* FIXME: check for inode number changes? */
> @@ -671,8 +676,6 @@ static int cifs_get_name_from_search_buf(struct qstr *pqst,
>                pqst->name = filename;
>                pqst->len = len;
>        }
> -       pqst->hash = full_name_hash(pqst->name, pqst->len);
> -/*     cFYI(1, ("filldir on %s",pqst->name));  */
>        return rc;
>  }
>
> --
> 1.6.6
>
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>

Jeff, these are functions, full_name_hash etc. treat name->name and
name->len as
character names strings and character name string lenghts, is that correct?
There is not unicode consideration while generating hash values at this point?
Jeff Layton - Feb. 5, 2010, 9:03 p.m.
On Fri, 5 Feb 2010 14:31:50 -0600
Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote:

> On Fri, Feb 5, 2010 at 12:30 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > case-insensitive mounts shouldn't use full_name_hash(). Make sure we
> > use the parent dentry's d_hash routine when one is set.
> >
> > Reported-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/readdir.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> > index f5618f8..c343b14 100644
> > --- a/fs/cifs/readdir.c
> > +++ b/fs/cifs/readdir.c
> > @@ -77,6 +77,11 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
> >
> >        cFYI(1, ("For %s", name->name));
> >
> > +       if (parent->d_op && parent->d_op->d_hash)
> > +               parent->d_op->d_hash(parent, name);
> > +       else
> > +               name->hash = full_name_hash(name->name, name->len);
> > +
> >        dentry = d_lookup(parent, name);
> >        if (dentry) {
> >                /* FIXME: check for inode number changes? */
> > @@ -671,8 +676,6 @@ static int cifs_get_name_from_search_buf(struct qstr *pqst,
> >                pqst->name = filename;
> >                pqst->len = len;
> >        }
> > -       pqst->hash = full_name_hash(pqst->name, pqst->len);
> > -/*     cFYI(1, ("filldir on %s",pqst->name));  */
> >        return rc;
> >  }
> >
> > --
> > 1.6.6
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > linux-cifs-client@lists.samba.org
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> >
> 
> Jeff, these are functions, full_name_hash etc. treat name->name and
> name->len as
> character names strings and character name string lenghts, is that correct?
> There is not unicode consideration while generating hash values at this point?

I'm not sure I fully understand your question...

The names are assumed to already be converted to filenames in the local
character set. The hashing routines just treat the string as an opaque
sequence of bytes of the given length.
Shirish Pargaonkar - Feb. 5, 2010, 9:23 p.m.
On Fri, Feb 5, 2010 at 3:03 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Fri, 5 Feb 2010 14:31:50 -0600
> Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote:
>
>> On Fri, Feb 5, 2010 at 12:30 PM, Jeff Layton <jlayton@redhat.com> wrote:
>> > case-insensitive mounts shouldn't use full_name_hash(). Make sure we
>> > use the parent dentry's d_hash routine when one is set.
>> >
>> > Reported-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> > ---
>> >  fs/cifs/readdir.c |    7 +++++--
>> >  1 files changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
>> > index f5618f8..c343b14 100644
>> > --- a/fs/cifs/readdir.c
>> > +++ b/fs/cifs/readdir.c
>> > @@ -77,6 +77,11 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
>> >
>> >        cFYI(1, ("For %s", name->name));
>> >
>> > +       if (parent->d_op && parent->d_op->d_hash)
>> > +               parent->d_op->d_hash(parent, name);
>> > +       else
>> > +               name->hash = full_name_hash(name->name, name->len);
>> > +
>> >        dentry = d_lookup(parent, name);
>> >        if (dentry) {
>> >                /* FIXME: check for inode number changes? */
>> > @@ -671,8 +676,6 @@ static int cifs_get_name_from_search_buf(struct qstr *pqst,
>> >                pqst->name = filename;
>> >                pqst->len = len;
>> >        }
>> > -       pqst->hash = full_name_hash(pqst->name, pqst->len);
>> > -/*     cFYI(1, ("filldir on %s",pqst->name));  */
>> >        return rc;
>> >  }
>> >
>> > --
>> > 1.6.6
>> >
>> > _______________________________________________
>> > linux-cifs-client mailing list
>> > linux-cifs-client@lists.samba.org
>> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
>> >
>>
>> Jeff, these are functions, full_name_hash etc. treat name->name and
>> name->len as
>> character names strings and character name string lenghts, is that correct?
>> There is not unicode consideration while generating hash values at this point?
>
> I'm not sure I fully understand your question...
>
> The names are assumed to already be converted to filenames in the local
> character set. The hashing routines just treat the string as an opaque
> sequence of bytes of the given length.
>
> --
> Jeff Layton <jlayton@redhat.com>
>

OK, understand it now. Looks fine.

Patch

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index f5618f8..c343b14 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -77,6 +77,11 @@  cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
 
 	cFYI(1, ("For %s", name->name));
 
+	if (parent->d_op && parent->d_op->d_hash)
+		parent->d_op->d_hash(parent, name);
+	else
+		name->hash = full_name_hash(name->name, name->len);
+
 	dentry = d_lookup(parent, name);
 	if (dentry) {
 		/* FIXME: check for inode number changes? */
@@ -671,8 +676,6 @@  static int cifs_get_name_from_search_buf(struct qstr *pqst,
 		pqst->name = filename;
 		pqst->len = len;
 	}
-	pqst->hash = full_name_hash(pqst->name, pqst->len);
-/*	cFYI(1, ("filldir on %s",pqst->name));  */
 	return rc;
 }