Re: [PATCH] vfat+affs case preservation

Jan Kratochvil (rcpt-linux-kernel.AT.vger.kernel.org@jankratochvil.net)
Wed, 2 Jul 2003 20:28:18 +0200


Hi,

On Wed, 02 Jul 2003 14:04:27 +0200, viro@parcelfarce.linux.theplanet.co.uk wrote:
...
> Obvious example of bad behaviour:

Thanks, each directory really must be represented by at most one dentry.
Updated patch changes order of vfs_rename() dentry locking from (old,new)
to (new,old) to get the 'new' filename in the target letter-case even for
directory entries.

Unfortunately rename("dir","DIR") will be nop if some file in dir/ is currently
open ("dir" dentry in use). This is not a serious issue for Samba-over-vfat.
One way would be to use target filename string instead of target dentry in
inode_operations->rename() although it would need updating of all filesystems.

http://www.jankratochvil.net/priv/vfat/linux-2.4.22-pre2-vfat7.diff
http://www.jankratochvil.net/priv/vfat/linux-2.5.73-bk10-vfat7.diff

Lace

linux-2.5.73-bk10-vfat7.diff:

diff -u -ru linux-2.5.73-bk10-orig/Documentation/filesystems/vfs.txt linux-2.5.73-bk10-vfat7/Documentation/filesystems/vfs.txt
--- linux-2.5.73-bk10-orig/Documentation/filesystems/vfs.txt Tue May 27 03:00:43 2003
+++ linux-2.5.73-bk10-vfat7/Documentation/filesystems/vfs.txt Mon Jun 30 18:45:06 2003
@@ -419,6 +419,7 @@
int (*d_revalidate)(struct dentry *);
int (*d_hash) (struct dentry *, struct qstr *);
int (*d_compare) (struct dentry *, struct qstr *, struct qstr *);
+ int (*d_compare_rename) (struct dentry *, struct qstr *, struct qstr *);
void (*d_delete)(struct dentry *);
void (*d_release)(struct dentry *);
void (*d_iput)(struct dentry *, struct inode *);
@@ -431,7 +432,13 @@

d_hash: called when the VFS adds a dentry to the hash table

- d_compare: called when a dentry should be compared with another
+ d_compare: called when a dentry should be compared with another.
+ Case-sensitive for all case-preserving filesystems no matter if the
+ filesystem structure is case-sensitive itself
+
+ d_compare_rename: compare equivalency of two dentries no matter their case.
+ This method is optional - useful for case-insensitive case-preserving
+ filesystems. dentries are simply compared by d_compare if not provided

d_delete: called when the last reference to a dentry is
deleted. This means no-one is using the dentry, however it is
diff -u -ru linux-2.5.73-bk10-orig/fs/affs/namei.c linux-2.5.73-bk10-vfat7/fs/affs/namei.c
--- linux-2.5.73-bk10-orig/fs/affs/namei.c Tue May 27 03:00:27 2003
+++ linux-2.5.73-bk10-vfat7/fs/affs/namei.c Wed Jul 2 19:23:37 2003
@@ -25,24 +25,35 @@

extern struct inode_operations affs_symlink_inode_operations;

-static int affs_toupper(int ch);
-static int affs_hash_dentry(struct dentry *, struct qstr *);
-static int affs_compare_dentry(struct dentry *, struct qstr *, struct qstr *);
-static int affs_intl_toupper(int ch);
-static int affs_intl_hash_dentry(struct dentry *, struct qstr *);
-static int affs_intl_compare_dentry(struct dentry *, struct qstr *, struct qstr *);
+static int affs_strict_toupper(int ch);
+static int affs_toupper(int ch);
+static int affs_intl_toupper(int ch);
+static int affs_hash_strictcase_dentry(struct dentry *, struct qstr *);
+static int affs_compare_strictcase_dentry(struct dentry *, struct qstr *, struct qstr *);
+static int affs_compare_anycase_dentry(struct dentry *, struct qstr *, struct qstr *);
+static int affs_intl_compare_anycase_dentry(struct dentry *, struct qstr *, struct qstr *);

struct dentry_operations affs_dentry_operations = {
- .d_hash = affs_hash_dentry,
- .d_compare = affs_compare_dentry,
+ .d_hash = affs_hash_strictcase_dentry,
+ .d_compare = affs_compare_strictcase_dentry,
+ .d_compare_rename = affs_compare_anycase_dentry,
};

struct dentry_operations affs_intl_dentry_operations = {
- .d_hash = affs_intl_hash_dentry,
- .d_compare = affs_intl_compare_dentry,
+ .d_hash = affs_hash_strictcase_dentry,
+ .d_compare = affs_compare_strictcase_dentry,
+ .d_compare_rename = affs_intl_compare_anycase_dentry,
};


+/* toupper() for case-sensitive dentries matching */
+
+static int
+affs_strict_toupper(int ch)
+{
+ return ch;
+}
+
/* Simple toupper() for DOS\1 */

static int
@@ -91,14 +102,9 @@
}

static int
-affs_hash_dentry(struct dentry *dentry, struct qstr *qstr)
-{
- return __affs_hash_dentry(dentry, qstr, affs_toupper);
-}
-static int
-affs_intl_hash_dentry(struct dentry *dentry, struct qstr *qstr)
+affs_hash_strictcase_dentry(struct dentry *dentry, struct qstr *qstr)
{
- return __affs_hash_dentry(dentry, qstr, affs_intl_toupper);
+ return __affs_hash_dentry(dentry, qstr, affs_strict_toupper);
}

static inline int
@@ -134,12 +140,17 @@
}

static int
-affs_compare_dentry(struct dentry *dentry, struct qstr *a, struct qstr *b)
+affs_compare_strictcase_dentry(struct dentry *dentry, struct qstr *a, struct qstr *b)
+{
+ return __affs_compare_dentry(dentry, a, b, affs_strict_toupper);
+}
+static int
+affs_compare_anycase_dentry(struct dentry *dentry, struct qstr *a, struct qstr *b)
{
return __affs_compare_dentry(dentry, a, b, affs_toupper);
}
static int
-affs_intl_compare_dentry(struct dentry *dentry, struct qstr *a, struct qstr *b)
+affs_intl_compare_anycase_dentry(struct dentry *dentry, struct qstr *a, struct qstr *b)
{
return __affs_compare_dentry(dentry, a, b, affs_intl_toupper);
}
@@ -226,6 +237,7 @@
}
if (bh) {
u32 ino = bh->b_blocknr;
+ struct dentry *alias;

/* store the real header ino in d_fsdata for faster lookups */
dentry->d_fsdata = (void *)(long)ino;
@@ -240,6 +252,15 @@
if (!inode) {
return ERR_PTR(-EACCES);
}
+ alias = d_find_alias(inode);
+ if (alias) {
+ if (d_invalidate(alias)==0)
+ dput(alias);
+ else {
+ iput(inode);
+ return alias;
+ }
+ }
}
dentry->d_op = AFFS_SB(sb)->s_flags & SF_INTL ? &affs_intl_dentry_operations : &affs_dentry_operations;
d_add(dentry, inode);
@@ -423,7 +444,11 @@
return retval;

/* Unlink destination if it already exists */
- if (new_dentry->d_inode) {
+ if (new_dentry->d_inode
+ /* Do not remove case-different aliases twice.
+ * Hardlinks cannot be passed here - checked at top of vfs_rename().
+ */
+ && old_dentry->d_inode != new_dentry->d_inode) {
retval = affs_remove_header(new_dentry);
if (retval)
return retval;
diff -u -ru linux-2.5.73-bk10-orig/fs/namei.c linux-2.5.73-bk10-vfat7/fs/namei.c
--- linux-2.5.73-bk10-orig/fs/namei.c Mon Jun 30 17:35:52 2003
+++ linux-2.5.73-bk10-vfat7/fs/namei.c Wed Jul 2 19:47:08 2003
@@ -1893,6 +1893,8 @@
return error;

target = new_dentry->d_inode;
+ if (old_dentry == new_dentry)
+ target = NULL;
if (target) {
down(&target->i_sem);
d_unhash(new_dentry);
@@ -1910,7 +1912,8 @@
dput(new_dentry);
}
if (!error) {
- d_move(old_dentry,new_dentry);
+ if (old_dentry != new_dentry)
+ d_move(old_dentry,new_dentry);
security_inode_post_rename(old_dir, old_dentry,
new_dir, new_dentry);
}
@@ -1937,7 +1940,8 @@
error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry);
if (!error) {
/* The following d_move() should become unconditional */
- if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME))
+ if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME)
+ && old_dentry != new_dentry)
d_move(old_dentry, new_dentry);
security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
}
@@ -1953,9 +1957,19 @@
int error;
int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);

- if (old_dentry->d_inode == new_dentry->d_inode)
- return 0;
-
+ /*
+ * Rename to the same inode but possibly different name.
+ * Pass such call only to case-insensitive case-preserving filesystems
+ * (vfat) implementing d_compare_rename. Other filesystems would crash
+ * as they do not expect rename to the same target inode.
+ * Renaming devoted to johanka.
+ */
+ if (old_dentry->d_inode == new_dentry->d_inode
+ && (!old_dentry->d_op || !old_dentry->d_op->d_compare_rename
+ || (old_dir != new_dir
+ || old_dentry->d_op->d_compare_rename(old_dentry, &old_dentry->d_name, &new_dentry->d_name))))
+ return 0;
+
error = may_delete(old_dir, old_dentry, is_dir);
if (error)
return error;
@@ -2019,41 +2033,46 @@

trap = lock_rename(new_dir, old_dir);

+ /*
+ * Lookup 'new' name first to get the target filename in the wanted
+ * letter-case on case-insensitive case-preserving filesystems.
+ * The case of 'old' name does not matter for these filesystems.
+ */
+ new_dentry = lookup_hash(&newnd.last, new_dir);
+ error = PTR_ERR(new_dentry);
+ if (IS_ERR(new_dentry))
+ goto exit3;
+ /* source should not be ancestor of target if not case-change rename */
+ error = -ENOTEMPTY;
+ if (new_dentry == trap && old_dir != new_dir)
+ goto exit4;
old_dentry = lookup_hash(&oldnd.last, old_dir);
error = PTR_ERR(old_dentry);
if (IS_ERR(old_dentry))
- goto exit3;
+ goto exit4;
/* source must exist */
error = -ENOENT;
if (!old_dentry->d_inode)
- goto exit4;
+ goto exit5;
/* unless the source is a directory trailing slashes give -ENOTDIR */
if (!S_ISDIR(old_dentry->d_inode->i_mode)) {
error = -ENOTDIR;
if (oldnd.last.name[oldnd.last.len])
- goto exit4;
+ goto exit5;
if (newnd.last.name[newnd.last.len])
- goto exit4;
+ goto exit5;
}
- /* source should not be ancestor of target */
+ /* source should not be ancestor of target if not case-change rename */
error = -EINVAL;
- if (old_dentry == trap)
- goto exit4;
- new_dentry = lookup_hash(&newnd.last, new_dir);
- error = PTR_ERR(new_dentry);
- if (IS_ERR(new_dentry))
- goto exit4;
- /* target should not be an ancestor of source */
- error = -ENOTEMPTY;
- if (new_dentry == trap)
+ if (old_dentry == trap && old_dir != new_dir)
goto exit5;

error = vfs_rename(old_dir->d_inode, old_dentry,
new_dir->d_inode, new_dentry);
exit5:
- dput(new_dentry);
-exit4:
dput(old_dentry);
+exit4:
+ dput(new_dentry);
exit3:
unlock_rename(new_dir, old_dir);
exit2:
diff -u -ru linux-2.5.73-bk10-orig/fs/vfat/namei.c linux-2.5.73-bk10-vfat7/fs/vfat/namei.c
--- linux-2.5.73-bk10-orig/fs/vfat/namei.c Mon Jun 30 17:35:12 2003
+++ linux-2.5.73-bk10-vfat7/fs/vfat/namei.c Wed Jul 2 19:53:24 2003
@@ -41,7 +41,6 @@
# define PRINTK3(x)
#endif

-static int vfat_hashi(struct dentry *parent, struct qstr *qstr);
static int vfat_hash(struct dentry *parent, struct qstr *qstr);
static int vfat_cmpi(struct dentry *dentry, struct qstr *a, struct qstr *b);
static int vfat_cmp(struct dentry *dentry, struct qstr *a, struct qstr *b);
@@ -49,22 +48,24 @@

static struct dentry_operations vfat_dentry_ops[4] = {
{
- .d_hash = vfat_hashi,
- .d_compare = vfat_cmpi,
+ .d_hash = vfat_hash,
+ .d_compare = vfat_cmp,
+ .d_compare_rename = vfat_cmpi,
},
{
- .d_revalidate = vfat_revalidate,
- .d_hash = vfat_hashi,
- .d_compare = vfat_cmpi,
+ .d_revalidate = vfat_revalidate,
+ .d_hash = vfat_hash,
+ .d_compare = vfat_cmp,
+ .d_compare_rename = vfat_cmpi,
},
{
- .d_hash = vfat_hash,
- .d_compare = vfat_cmp,
+ .d_hash = vfat_hash,
+ .d_compare = vfat_cmp,
},
{
- .d_revalidate = vfat_revalidate,
- .d_hash = vfat_hash,
- .d_compare = vfat_cmp,
+ .d_revalidate = vfat_revalidate,
+ .d_hash = vfat_hash,
+ .d_compare = vfat_cmp,
}
};

@@ -129,32 +130,6 @@
}

/*
- * Compute the hash for the vfat name corresponding to the dentry.
- * Note: if the name is invalid, we leave the hash code unchanged so
- * that the existing dentry can be used. The vfat fs routines will
- * return ENOENT or EINVAL as appropriate.
- */
-static int vfat_hashi(struct dentry *dentry, struct qstr *qstr)
-{
- struct nls_table *t = MSDOS_SB(dentry->d_inode->i_sb)->nls_io;
- const char *name;
- int len;
- unsigned long hash;
-
- len = qstr->len;
- name = qstr->name;
- while (len && name[len-1] == '.')
- len--;
-
- hash = init_name_hash();
- while (len--)
- hash = partial_name_hash(vfat_tolower(t, *name++), hash);
- qstr->hash = end_name_hash(hash);
-
- return 0;
-}
-
-/*
* Case insensitive compare of two vfat names.
*/
static int vfat_cmpi(struct dentry *dentry, struct qstr *a, struct qstr *b)
@@ -1082,11 +1057,14 @@
loff_t dotdot_i_pos;
struct inode *old_inode, *new_inode;
int res, is_dir;
- struct vfat_slot_info old_sinfo,sinfo;
+ struct vfat_slot_info old_sinfo,new_sinfo;

old_bh = new_bh = dotdot_bh = NULL;
old_inode = old_dentry->d_inode;
new_inode = new_dentry->d_inode;
+ /* Rename of case-different aliases in the same dir. */
+ if (old_inode == new_inode)
+ new_inode = NULL;
lock_kernel();
res = vfat_find(old_dir,&old_dentry->d_name,&old_sinfo,&old_bh,&old_de);
PRINTK3(("vfat_rename 2\n"));
@@ -1098,10 +1076,10 @@
&dotdot_de,&dotdot_i_pos)) < 0)
goto rename_done;

- if (new_dentry->d_inode) {
- res = vfat_find(new_dir,&new_dentry->d_name,&sinfo,&new_bh,
+ if (new_inode) {
+ res = vfat_find(new_dir,&new_dentry->d_name,&new_sinfo,&new_bh,
&new_de);
- if (res < 0 || MSDOS_I(new_inode)->i_pos != sinfo.i_pos) {
+ if (res < 0 || MSDOS_I(new_inode)->i_pos != new_sinfo.i_pos) {
/* WTF??? Cry and fail. */
printk(KERN_WARNING "vfat_rename: fs corrupted\n");
goto rename_done;
@@ -1112,11 +1090,10 @@
if (res)
goto rename_done;
}
+ /* releases new_bh */
+ vfat_remove_entry(new_dir,&new_sinfo,new_bh,new_de);
+ new_bh=NULL;
fat_detach(new_inode);
- } else {
- res = vfat_add_entry(new_dir,&new_dentry->d_name,is_dir,&sinfo,
- &new_bh,&new_de);
- if (res < 0) goto rename_done;
}

new_dir->i_version++;
@@ -1124,8 +1101,12 @@
/* releases old_bh */
vfat_remove_entry(old_dir,&old_sinfo,old_bh,old_de);
old_bh=NULL;
+ res = vfat_add_entry(new_dir,&new_dentry->d_name,is_dir,&new_sinfo,
+ &new_bh,&new_de);
+ if (res < 0) goto rename_done;
+
fat_detach(old_inode);
- fat_attach(old_inode, sinfo.i_pos);
+ fat_attach(old_inode, new_sinfo.i_pos);
mark_inode_dirty(old_inode);

old_dir->i_version++;
diff -u -ru linux-2.5.73-bk10-orig/include/linux/dcache.h linux-2.5.73-bk10-vfat7/include/linux/dcache.h
--- linux-2.5.73-bk10-orig/include/linux/dcache.h Mon Jun 30 17:35:17 2003
+++ linux-2.5.73-bk10-vfat7/include/linux/dcache.h Mon Jun 30 18:44:33 2003
@@ -109,6 +109,7 @@
int (*d_revalidate)(struct dentry *, int);
int (*d_hash) (struct dentry *, struct qstr *);
int (*d_compare) (struct dentry *, struct qstr *, struct qstr *);
+ int (*d_compare_rename) (struct dentry *, struct qstr *, struct qstr *);
int (*d_delete)(struct dentry *);
void (*d_release)(struct dentry *);
void (*d_iput)(struct dentry *, struct inode *);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/