| 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: <874nl4mee3.fsf@devron.myhome.or.jp>
Date: Mon, 05 Nov 2012 21:18:44 +0900
From: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To: Namjae Jeon <linkinjeon@...il.com>
Cc: akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org,
Namjae Jeon <namjae.jeon@...sung.com>,
Ravishankar N <ravi.n1@...sung.com>,
Amit Sahrawat <a.sahrawat@...sung.com>
Subject: Re: [PATCH 2/5] fat: restructure export operations
Namjae Jeon <linkinjeon@...il.com> writes:
> +#define FILEID_FAT_WITHOUT_PARENT (offsetof(struct fat_fid, parent_i_pos_hi)/4)
> +#define FILEID_FAT_WITH_PARENT (sizeof(struct fat_fid)/4)
This is strange.
FILEID_FAT_WITH_PARENT and FILEID_FAT_WITHOUT_PARENT should be fh_type,
not length.
> int
> -fat_encode_fh(struct inode *inode, __u32 *fh, int *lenp, struct inode *parent)
> +fat_encode_fh_nostale(struct inode *inode, __u32 *fh, int *lenp,
> + struct inode *parent)
> {
> int len = *lenp;
> struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
> @@ -97,24 +111,26 @@ fat_encode_fh(struct inode *inode, __u32 *fh, int *lenp, struct inode *parent)
> loff_t i_pos;
> int type = FILEID_INO32_GEN;
Please don't re-use FILEID_INO32_GEN(_PARENT) for different FH
format. Instead of those, we should use FILEID_FAT_(WITH|WITHOUT)_PARENT
here.
> - if (parent && (len < 5)) {
> - *lenp = 5;
> + if (parent && (len < FILEID_FAT_WITH_PARENT)) {
> + *lenp = FILEID_FAT_WITH_PARENT;
> return 255;
> - } else if (len < 3) {
> - *lenp = 3;
> + } else if (len < FILEID_FAT_WITHOUT_PARENT) {
> + *lenp = FILEID_FAT_WITHOUT_PARENT;
> return 255;
> }
>
> i_pos = fat_i_pos_read(sbi, inode);
> - *lenp = 3;
> - fid->ino = inode->i_ino;
> - fid->gen = inode->i_generation;
> - fid->i_pos = i_pos;
> + *lenp = FILEID_FAT_WITHOUT_PARENT;
> + fid->i_gen = inode->i_generation;
> + fid->i_pos_low = i_pos & 0xFFFFFFFF;
> + fid->i_pos_hi = (i_pos >> 32) & 0xFF;
0xff is 0xffff actually?
> if (parent) {
> - fid->parent_ino = parent->i_ino;
> - fid->parent_gen = parent->i_generation;
> + i_pos = fat_i_pos_read(sbi, parent);
> + fid->parent_i_pos_hi = (i_pos >> 32) & 0xFF;
> + fid->parent_i_pos_low = i_pos & 0xFFFFFFFF;
> + fid->parent_i_gen = parent->i_generation;
> type = FILEID_INO32_GEN_PARENT;
> - *lenp = 5;
> + *lenp = FILEID_FAT_WITH_PARENT;
> }
>
> return type;
> @@ -128,14 +144,36 @@ struct dentry *fat_fh_to_dentry(struct super_block *sb, struct fid *fh,
> int fh_len, int fh_type)
> {
> struct inode *inode = NULL;
> - struct fat_fid *fid = (struct fat_fid *)fh;
> - if (fh_len < 3)
> + if (fh_len < 2)
> return NULL;
>
> switch (fh_type) {
> case FILEID_INO32_GEN:
> case FILEID_INO32_GEN_PARENT:
> - inode = fat_nfs_get_inode(sb, fid->ino, fid->gen, fid->i_pos);
> + inode = fat_nfs_get_inode(sb, fh->i32.ino, fh->i32.gen, 0);
> + break;
> + }
We can use generic_fh_to_dentry() here, instead, opencode. Right?
Rename fat_nfs_get_inode() to __fat_nfs_get_inode(). And add
fat_nfs_get_inode(struct super_block *sb, u64 ino, u32 gen)
{
return __fat_nfs_get_inode(sb, fh->i32.ino, fh->i32.gen, 0);
}
> + switch (fh_type) {
> + case FILEID_INO32_GEN:
> + if (fh_len < FILEID_FAT_WITHOUT_PARENT)
> + return NULL;
> + case FILEID_INO32_GEN_PARENT:
> + if ((fh_len < FILEID_FAT_WITH_PARENT) &&
> + (fh_type == FILEID_INO32_GEN_PARENT))
> + return NULL;
Why do we have to care unused fields here?
> + i_pos = fid->i_pos_hi;
> + i_pos = (i_pos << 32) | (fid->i_pos_low);
> + inode = fat_nfs_get_inode(sb, 0, fid->i_gen, i_pos);
> break;
> }
> @@ -147,18 +185,39 @@ struct dentry *fat_fh_to_dentry(struct super_block *sb, struct fid *fh,
> * Find the parent for a file specified by NFS handle.
> * This requires that the handle contain the i_ino of the parent.
> */
> -struct dentry *fat_fh_to_parent(struct super_block *sb, struct fid *fh,
> +struct dentry *fat_fh_to_parent(struct super_block *sb, struct fid *fid,
> + int fh_len, int fh_type)
> +{
> + struct inode *inode = NULL;
> +
> + if (fh_len < 2)
> + return NULL;
> +
> + switch (fh_type) {
> + case FILEID_INO32_GEN:
> + case FILEID_INO32_GEN_PARENT:
> + inode = fat_nfs_get_inode(sb, fid->i32.ino, fid->i32.gen, 0);
> + break;
> + }
Don't opencode generic_fh_to_parent() here too. And this is wrong.
> +struct dentry *fat_fh_to_parent_nostale(struct super_block *sb, struct fid *fh,
> int fh_len, int fh_type)
> {
> struct inode *inode = NULL;
> struct fat_fid *fid = (struct fat_fid *)fh;
> - if (fh_len < 5)
> + loff_t i_pos;
> +
> + if (fh_len < FILEID_FAT_WITH_PARENT)
> return NULL;
>
> switch (fh_type) {
> case FILEID_INO32_GEN_PARENT:
> - inode = fat_nfs_get_inode(sb, fid->parent_ino, fid->parent_gen,
> - fid->i_pos);
> + i_pos = fid->parent_i_pos_hi;
> + i_pos = (i_pos << 32) | (fid->parent_i_pos_low);
> + inode = fat_nfs_get_inode(sb, 0, fid->parent_i_gen, i_pos);
> break;
> }
>
> @@ -303,3 +362,15 @@ out:
>
> return d_obtain_alias(parent_inode);
> }
> +
> +const struct export_operations fat_export_ops = {
> + .fh_to_dentry = fat_fh_to_dentry,
> + .fh_to_parent = fat_fh_to_parent,
> + .get_parent = fat_get_parent,
> +};
Need empty line here.
> +const struct export_operations fat_export_ops_nostale = {
> + .encode_fh = fat_encode_fh_nostale,
> + .fh_to_dentry = fat_fh_to_dentry_nostale,
> + .fh_to_parent = fat_fh_to_parent_nostale,
> + .get_parent = fat_get_parent,
> +};
--
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists