On 3/4/06, Phillip Susi <[email protected]> wrote: > This patch fixes a bug in udf where it would write uid/gid = 0 to the > disk for files owned by the id given with the uid=/gid= mount options. I don't think it does. You can still get zero uid/gid written on disk if you specify uid/gid mount options when not specifying uid=forget. I am not against new features but lets fix the bug first, okay? Adding new mount options to work around buggy code is not the right solution. The unconditional memset() is clearly a regression caused by Ben Fennema's commit "[PATCH] UDF sync with CVS" in the old-2.6-bkcvs.git repository and should be fixed. Care to try out the included untested patch to see if it fixes the problem? (Sorry for the attachment, I am replying via web mail.) Pekka [PATCH] udf: fix uid/gid changing to zero This patch fixes uid/gid changing to zero when uid/gid mount option is used for UDF. The problem appears when the overriding uid/gid matches inode. The fix is simple, instead of unconditionally zeroing buffer, we now only do it for new inodes. The regression appeared in commit "[PATCH] UDF sync with CVS" and was first reported by Phillip Susi. Cc: Phillip Susi <[email protected]> Signed-off-by: Pekka Enberg <[email protected]> --- fs/udf/ialloc.c | 1 + fs/udf/inode.c | 9 +++++++-- fs/udf/udf_i.h | 1 + include/linux/udf_fs_i.h | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c index c9b707b..4bc3dc9 100644 --- a/fs/udf/ialloc.c +++ b/fs/udf/ialloc.c @@ -146,6 +146,7 @@ struct inode * udf_new_inode (struct ino UDF_I_ALLOCTYPE(inode) = ICBTAG_FLAG_AD_LONG; inode->i_mtime = inode->i_atime = inode->i_ctime = UDF_I_CRTIME(inode) = current_fs_time(inode->i_sb); + UDF_I_NEW_INODE(inode) = 1; insert_inode_hash(inode); mark_inode_dirty(inode); up(&sbi->s_alloc_sem); diff --git a/fs/udf/inode.c b/fs/udf/inode.c index 395e582..6937da2 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -1003,6 +1003,8 @@ static void udf_fill_inode(struct inode long convtime_usec; int offset; + UDF_I_NEW_INODE(inode) = 0; + fe = (struct fileEntry *)bh->b_data; efe = (struct extendedFileEntry *)bh->b_data; @@ -1307,11 +1309,14 @@ udf_update_inode(struct inode *inode, in return -EIO; } - memset(bh->b_data, 0x00, inode->i_sb->s_blocksize); - fe = (struct fileEntry *)bh->b_data; efe = (struct extendedFileEntry *)bh->b_data; + if (UDF_I_NEW_INODE(inode) == 1) { + memset(bh->b_data, 0x00, inode->i_sb->s_blocksize); + UDF_I_NEW_INODE(inode) = 0; + } + if (le16_to_cpu(fe->descTag.tagIdent) == TAG_IDENT_USE) { struct unallocSpaceEntry *use = diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h index d7dbe6f..87f82f7 100644 --- a/fs/udf/udf_i.h +++ b/fs/udf/udf_i.h @@ -16,6 +16,7 @@ static inline struct udf_inode_info *UDF #define UDF_I_EFE(X) ( UDF_I(X)->i_efe ) #define UDF_I_USE(X) ( UDF_I(X)->i_use ) #define UDF_I_STRAT4096(X) ( UDF_I(X)->i_strat4096 ) +#define UDF_I_NEW_INODE(X) ( UDF_I(X)->i_new_inode ) #define UDF_I_NEXT_ALLOC_BLOCK(X) ( UDF_I(X)->i_next_alloc_block ) #define UDF_I_NEXT_ALLOC_GOAL(X) ( UDF_I(X)->i_next_alloc_goal ) #define UDF_I_CRTIME(X) ( UDF_I(X)->i_crtime ) diff --git a/include/linux/udf_fs_i.h b/include/linux/udf_fs_i.h index 1e75084..c21415a 100644 --- a/include/linux/udf_fs_i.h +++ b/include/linux/udf_fs_i.h @@ -51,7 +51,8 @@ struct udf_inode_info unsigned i_efe : 1; unsigned i_use : 1; unsigned i_strat4096 : 1; - unsigned reserved : 26; + unsigned i_new_inode : 1; + unsigned reserved : 25; union { short_ad *i_sad;
Attachment:
udf-fix
Description: Binary data
- Follow-Ups:
- References:
- Prev by Date: Re: de2104x: interrupts before interrupt handler is registered
- Next by Date: Re: Ocfs2 performance bugs of doom
- Previous by thread: [PATCH] udf: fix uid/gid options and add uid/gid=ignore and forget options
- Next by thread: Re: [PATCH] udf: fix uid/gid options and add uid/gid=ignore and forget options
- Index(es):
 |