* chmod messes up permissions on hfs filesystem @ 2004-11-01 4:35 Horms 2004-11-01 11:05 ` Horms 2004-11-01 16:27 ` Roman Zippel 0 siblings, 2 replies; 7+ messages in thread From: Horms @ 2004-11-01 4:35 UTC (permalink / raw) To: LKML; +Cc: Roman Zippel, Siep Kroonenberg, 278068 On Sun, Oct 24, 2004 at 05:21:44PM +0200, Siep Kroonenberg wrote: > Package: kernel-image-2.6.8-powerpc Version: 2.6.8-6 Severity: normal > > > chmod commands on files on hfs partitions tend to give weird results, > e.g.: > > original: -rw-r--r-- after chmod g+w: -----w--w- after chmod g-w: > ---------- after unmounting and remounting the partition: -r--r--r-- > > I assume this is kernel-related, since with a 2.4 kernel, chmod > commands mostly got ignored on this hfs partition. Anyhow, the > maintainer of coreutils doesn't consider this a problem with chmod. That is very strange indeed. I have sent this on to LKML so see if he has any ideas. >From reading hfs_inode_setattr() in fs/hfs/inode.c I observe that, it only honours changes for the write flag, and it changes the global, group and user flag simultaneously. I guess HFS only has one permission flag, write, which can be on or off. The relevant code in the hfs tree doesn't seem to have changed for many moons, so either it has always been broken in 2.6 or something strange has happened elsewhere. In any case the behaviuor describe above is weird and should work more along the lines of. -rw-r--r-- after chmod g+w: -rw-rw-rw- after chmod g-w: -r--r--r-- after unmounting and remounting the partition: -r--r--r-- -- Horms ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-01 4:35 chmod messes up permissions on hfs filesystem Horms @ 2004-11-01 11:05 ` Horms 2004-11-01 16:27 ` Roman Zippel 1 sibling, 0 replies; 7+ messages in thread From: Horms @ 2004-11-01 11:05 UTC (permalink / raw) To: LKML; +Cc: Roman Zippel, Siep Kroonenberg, 278068 On Mon, Nov 01, 2004 at 01:35:59PM +0900, Horms wrote: > On Sun, Oct 24, 2004 at 05:21:44PM +0200, Siep Kroonenberg wrote: > > Package: kernel-image-2.6.8-powerpc Version: 2.6.8-6 Severity: normal > > > > > > chmod commands on files on hfs partitions tend to give weird results, > > e.g.: > > > > original: -rw-r--r-- > > after chmod g+w: > > -----w--w- > > after chmod g-w: > > ---------- > > after unmounting and remounting the partition: > > -r--r--r-- > > > > I assume this is kernel-related, since with a 2.4 kernel, chmod > > commands mostly got ignored on this hfs partition. Anyhow, the > > maintainer of coreutils doesn't consider this a problem with chmod. > > That is very strange indeed. I have sent this on to LKML so see if he has > any ideas. > > From reading hfs_inode_setattr() in fs/hfs/inode.c I observe that, it > only honours changes for the write flag, and it changes the global, > group and user flag simultaneously. I guess HFS only has one permission > flag, write, which can be on or off. The relevant code in the hfs tree > doesn't seem to have changed for many moons, so either it has always > been broken in 2.6 or something strange has happened elsewhere. > > In any case the behaviuor describe above is weird and should work > more along the lines of. > > -rw-r--r-- > after chmod g+w: > -rw-rw-rw- > after chmod g-w: > -r--r--r-- > after unmounting and remounting the partition: > -r--r--r-- I took a closer look into this and it seems to be caused by completely bogus handling of the umask and dirmask of the fs which is set at mount time and supposed to control the default permisions of files. I am working on a patch to resolve this. -- Horms ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-01 4:35 chmod messes up permissions on hfs filesystem Horms 2004-11-01 11:05 ` Horms @ 2004-11-01 16:27 ` Roman Zippel 2004-11-02 3:56 ` Horms 1 sibling, 1 reply; 7+ messages in thread From: Roman Zippel @ 2004-11-01 16:27 UTC (permalink / raw) To: Horms; +Cc: LKML, Siep Kroonenberg, 278068 Hi, On Mon, 1 Nov 2004, Horms wrote: > -rw-r--r-- > after chmod g+w: > -rw-rw-rw- No, this command will do nothing, as only the owner write bit is checked and the owner bit is mirrored in the other parts (minus umask). Below is a patch which should fix the original problem. bye, Roman Index: fs/hfs/inode.c =================================================================== RCS file: /home/other/cvs/linux/linux-2.6/fs/hfs/inode.c,v retrieving revision 1.1.1.5 diff -u -p -r1.1.1.5 inode.c --- fs/hfs/inode.c 17 Jun 2004 08:39:32 -0000 1.1.1.5 +++ fs/hfs/inode.c 1 Nov 2004 16:14:50 -0000 @@ -313,7 +313,7 @@ int hfs_read_inode(struct inode *inode, inode->i_mode = S_IRUGO | S_IXUGO; if (!(rec->file.Flags & HFS_FIL_LOCK)) inode->i_mode |= S_IWUGO; - inode->i_mode &= hsb->s_file_umask; + inode->i_mode &= ~hsb->s_file_umask; inode->i_mode |= S_IFREG; inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->file.MdDat); @@ -326,7 +326,7 @@ int hfs_read_inode(struct inode *inode, inode->i_ino = be32_to_cpu(rec->dir.DirID); inode->i_blocks = 0; inode->i_size = be16_to_cpu(rec->dir.Val) + 2; - inode->i_mode = S_IFDIR | (S_IRWXUGO & hsb->s_dir_umask); + inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask); inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->file.MdDat); inode->i_op = &hfs_dir_inode_operations; Index: fs/hfs/super.c =================================================================== RCS file: /home/other/cvs/linux/linux-2.6/fs/hfs/super.c,v retrieving revision 1.1.1.5 diff -u -p -r1.1.1.5 super.c --- fs/hfs/super.c 10 May 2004 15:02:37 -0000 1.1.1.5 +++ fs/hfs/super.c 1 Nov 2004 16:03:14 -0000 @@ -150,8 +150,8 @@ static int parse_options(char *options, /* initialize the sb with defaults */ hsb->s_uid = current->uid; hsb->s_gid = current->gid; - hsb->s_file_umask = 0644; - hsb->s_dir_umask = 0755; + hsb->s_file_umask = 0133; + hsb->s_dir_umask = 0022; hsb->s_type = 0x3f3f3f3f; /* == '????' */ hsb->s_creator = 0x3f3f3f3f; /* == '????' */ hsb->s_quiet = 0; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-01 16:27 ` Roman Zippel @ 2004-11-02 3:56 ` Horms 2004-11-03 16:00 ` Roman Zippel 0 siblings, 1 reply; 7+ messages in thread From: Horms @ 2004-11-02 3:56 UTC (permalink / raw) To: Roman Zippel; +Cc: LKML, Siep Kroonenberg, 278068 On Mon, Nov 01, 2004 at 05:27:47PM +0100, Roman Zippel wrote: > Hi, > > On Mon, 1 Nov 2004, Horms wrote: > > > -rw-r--r-- > > after chmod g+w: > > -rw-rw-rw- > > No, this command will do nothing, as only the owner write bit is checked > and the owner bit is mirrored in the other parts (minus umask). > Below is a patch which should fix the original problem. Hi, Thanks for the patch, though the behaviour of the umask still seems rather odd. I would like to offer an updated patch which I believe makes the umask behave in the expected way. It also ensures that the write_lock bit is read from/written to disk correctly. -- Horms ===== fs/hfs/inode.c 1.24 vs edited ===== --- 1.24/fs/hfs/inode.c 2004-10-26 05:06:48 +09:00 +++ edited/fs/hfs/inode.c 2004-11-01 20:56:45 +09:00 @@ -158,6 +158,7 @@ struct inode *hfs_new_inode(struct inode { struct super_block *sb = dir->i_sb; struct inode *inode = new_inode(sb); + struct hfs_sb_info *hsb = HFS_SB(dir->i_sb); if (!inode) return NULL; @@ -165,7 +166,6 @@ struct inode *hfs_new_inode(struct inode INIT_LIST_HEAD(&HFS_I(inode)->open_dir_list); hfs_cat_build_key((btree_key *)&HFS_I(inode)->cat_key, dir->i_ino, name); inode->i_ino = HFS_SB(sb)->next_id++; - inode->i_mode = mode; inode->i_uid = current->fsuid; inode->i_gid = current->fsgid; inode->i_nlink = 1; @@ -174,14 +174,15 @@ struct inode *hfs_new_inode(struct inode HFS_I(inode)->flags = 0; HFS_I(inode)->rsrc_inode = NULL; HFS_I(inode)->fs_blocks = 0; - if (S_ISDIR(inode->i_mode)) { + if (S_ISDIR(mode)) { inode->i_size = 2; HFS_SB(sb)->folder_count++; if (dir->i_ino == HFS_ROOT_CNID) HFS_SB(sb)->root_dirs++; inode->i_op = &hfs_dir_inode_operations; inode->i_fop = &hfs_dir_operations; - } else if (S_ISREG(inode->i_mode)) { + inode->i_mode = (mode & ~0777) | (~hsb->s_dir_umask & 0777); + } else if (S_ISREG(mode)) { HFS_I(inode)->clump_blocks = HFS_SB(sb)->clumpablks; HFS_SB(sb)->file_count++; if (dir->i_ino == HFS_ROOT_CNID) @@ -196,6 +197,11 @@ struct inode *hfs_new_inode(struct inode HFS_I(inode)->cached_blocks = 0; memset(HFS_I(inode)->first_extents, 0, sizeof(hfs_extent_rec)); memset(HFS_I(inode)->cached_extents, 0, sizeof(hfs_extent_rec)); + inode->i_mode = (mode & ~0777) | (~hsb->s_file_umask & 0777); + if (mode & S_IWUSR) + inode->i_mode |= S_IWUGO; + else + inode->i_mode &= ~S_IWUGO; } insert_inode_hash(inode); mark_inode_dirty(inode); @@ -314,10 +320,11 @@ int hfs_read_inode(struct inode *inode, } inode->i_ino = be32_to_cpu(rec->file.FlNum); - inode->i_mode = S_IRUGO | S_IXUGO; + inode->i_mode = S_IRWXUGO & ~hsb->s_file_umask; if (!(rec->file.Flags & HFS_FIL_LOCK)) inode->i_mode |= S_IWUGO; - inode->i_mode &= hsb->s_file_umask; + else + inode->i_mode &= ~S_IWUGO; inode->i_mode |= S_IFREG; inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->file.MdDat); @@ -329,7 +336,7 @@ int hfs_read_inode(struct inode *inode, inode->i_ino = be32_to_cpu(rec->dir.DirID); inode->i_size = be16_to_cpu(rec->dir.Val) + 2; HFS_I(inode)->fs_blocks = 0; - inode->i_mode = S_IFDIR | (S_IRWXUGO & hsb->s_dir_umask); + inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask); inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->dir.MdDat); inode->i_op = &hfs_dir_inode_operations; @@ -601,7 +608,6 @@ int hfs_inode_setattr(struct dentry *den attr->ia_mode = inode->i_mode | S_IWUGO; else attr->ia_mode = inode->i_mode & ~S_IWUGO; - attr->ia_mode &= S_ISDIR(inode->i_mode) ? ~hsb->s_dir_umask: ~hsb->s_file_umask; } error = inode_setattr(inode, attr); if (error) ===== fs/hfs/super.c 1.32 vs edited ===== --- 1.32/fs/hfs/super.c 2004-10-26 05:06:47 +09:00 +++ edited/fs/hfs/super.c 2004-11-01 20:01:54 +09:00 @@ -149,8 +149,8 @@ static int parse_options(char *options, /* initialize the sb with defaults */ hsb->s_uid = current->uid; hsb->s_gid = current->gid; - hsb->s_file_umask = 0644; - hsb->s_dir_umask = 0755; + hsb->s_file_umask = 0111; + hsb->s_dir_umask = 0000; hsb->s_type = hsb->s_creator = cpu_to_be32(0x3f3f3f3f); /* == '????' */ hsb->s_quiet = 0; hsb->part = -1; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-02 3:56 ` Horms @ 2004-11-03 16:00 ` Roman Zippel 2004-11-04 3:31 ` Horms 0 siblings, 1 reply; 7+ messages in thread From: Roman Zippel @ 2004-11-03 16:00 UTC (permalink / raw) To: Horms; +Cc: LKML, Siep Kroonenberg, 278068 Hi, On Tue, 2 Nov 2004, Horms wrote: > Thanks for the patch, though the behaviour of the umask still seems > rather odd. I would like to offer an updated patch which I believe > makes the umask behave in the expected way. It also ensures > that the write_lock bit is read from/written to disk correctly. You apply the umask before updating the write bit, which is incorrect. > @@ -196,6 +197,11 @@ struct inode *hfs_new_inode(struct inode > HFS_I(inode)->cached_blocks = 0; > memset(HFS_I(inode)->first_extents, 0, sizeof(hfs_extent_rec)); > memset(HFS_I(inode)->cached_extents, 0, sizeof(hfs_extent_rec)); > + inode->i_mode = (mode & ~0777) | (~hsb->s_file_umask & 0777); > + if (mode & S_IWUSR) > + inode->i_mode |= S_IWUGO; > + else > + inode->i_mode &= ~S_IWUGO; > } > insert_inode_hash(inode); > mark_inode_dirty(inode); Thanks, for reminding me to fix hfs_new_inode here, but the above applies. > ===== fs/hfs/super.c 1.32 vs edited ===== > --- 1.32/fs/hfs/super.c 2004-10-26 05:06:47 +09:00 > +++ edited/fs/hfs/super.c 2004-11-01 20:01:54 +09:00 > @@ -149,8 +149,8 @@ static int parse_options(char *options, > /* initialize the sb with defaults */ > hsb->s_uid = current->uid; > hsb->s_gid = current->gid; > - hsb->s_file_umask = 0644; > - hsb->s_dir_umask = 0755; > + hsb->s_file_umask = 0111; > + hsb->s_dir_umask = 0000; > hsb->s_type = hsb->s_creator = cpu_to_be32(0x3f3f3f3f); /* == '????' */ > hsb->s_quiet = 0; > hsb->part = -1; This may be closer to the mac default, where everyone can access anything, but I'd rather keep a safe default. Below is my updated patch. bye, Roman Index: fs/hfs/inode.c =================================================================== RCS file: /home/other/cvs/linux/linux-2.6/fs/hfs/inode.c,v retrieving revision 1.1.1.5 diff -u -p -r1.1.1.5 inode.c --- fs/hfs/inode.c 17 Jun 2004 08:39:32 -0000 1.1.1.5 +++ fs/hfs/inode.c 3 Nov 2004 15:51:43 -0000 @@ -172,14 +172,16 @@ struct inode *hfs_new_inode(struct inode inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME; HFS_I(inode)->flags = 0; HFS_I(inode)->rsrc_inode = NULL; - if (S_ISDIR(inode->i_mode)) { + if (S_ISDIR(mode)) { inode->i_size = 2; HFS_SB(sb)->folder_count++; if (dir->i_ino == HFS_ROOT_CNID) HFS_SB(sb)->root_dirs++; inode->i_op = &hfs_dir_inode_operations; inode->i_fop = &hfs_dir_operations; - } else if (S_ISREG(inode->i_mode)) { + inode->i_mode |= S_IRWXUGO; + inode->i_mode &= ~HFS_SB(inode->i_sb)->s_dir_umask; + } else if (S_ISREG(mode)) { HFS_I(inode)->clump_blocks = HFS_SB(sb)->clumpablks; HFS_SB(sb)->file_count++; if (dir->i_ino == HFS_ROOT_CNID) @@ -187,6 +189,10 @@ struct inode *hfs_new_inode(struct inode inode->i_op = &hfs_file_inode_operations; inode->i_fop = &hfs_file_operations; inode->i_mapping->a_ops = &hfs_aops; + inode->i_mode |= S_IRUGO|S_IXUGO; + if (mode & S_IWUSR) + inode->i_mode |= S_IWUGO; + inode->i_mode &= ~HFS_SB(inode->i_sb)->s_file_umask; HFS_I(inode)->phys_size = 0; HFS_I(inode)->alloc_blocks = 0; HFS_I(inode)->first_blocks = 0; @@ -313,7 +319,7 @@ int hfs_read_inode(struct inode *inode, inode->i_mode = S_IRUGO | S_IXUGO; if (!(rec->file.Flags & HFS_FIL_LOCK)) inode->i_mode |= S_IWUGO; - inode->i_mode &= hsb->s_file_umask; + inode->i_mode &= ~hsb->s_file_umask; inode->i_mode |= S_IFREG; inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->file.MdDat); @@ -326,7 +332,7 @@ int hfs_read_inode(struct inode *inode, inode->i_ino = be32_to_cpu(rec->dir.DirID); inode->i_blocks = 0; inode->i_size = be16_to_cpu(rec->dir.Val) + 2; - inode->i_mode = S_IFDIR | (S_IRWXUGO & hsb->s_dir_umask); + inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask); inode->i_ctime = inode->i_atime = inode->i_mtime = hfs_m_to_utime(rec->file.MdDat); inode->i_op = &hfs_dir_inode_operations; Index: fs/hfs/super.c =================================================================== RCS file: /home/other/cvs/linux/linux-2.6/fs/hfs/super.c,v retrieving revision 1.1.1.5 diff -u -p -r1.1.1.5 super.c --- fs/hfs/super.c 10 May 2004 15:02:37 -0000 1.1.1.5 +++ fs/hfs/super.c 1 Nov 2004 16:03:14 -0000 @@ -150,8 +150,8 @@ static int parse_options(char *options, /* initialize the sb with defaults */ hsb->s_uid = current->uid; hsb->s_gid = current->gid; - hsb->s_file_umask = 0644; - hsb->s_dir_umask = 0755; + hsb->s_file_umask = 0133; + hsb->s_dir_umask = 0022; hsb->s_type = 0x3f3f3f3f; /* == '????' */ hsb->s_creator = 0x3f3f3f3f; /* == '????' */ hsb->s_quiet = 0; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-03 16:00 ` Roman Zippel @ 2004-11-04 3:31 ` Horms 2004-11-04 4:35 ` Horms 0 siblings, 1 reply; 7+ messages in thread From: Horms @ 2004-11-04 3:31 UTC (permalink / raw) To: Roman Zippel; +Cc: LKML, Siep Kroonenberg, 278068 On Wed, Nov 03, 2004 at 05:00:35PM +0100, Roman Zippel wrote: > Hi, > > On Tue, 2 Nov 2004, Horms wrote: > > > Thanks for the patch, though the behaviour of the umask still seems > > rather odd. I would like to offer an updated patch which I believe > > makes the umask behave in the expected way. It also ensures > > that the write_lock bit is read from/written to disk correctly. > > You apply the umask before updating the write bit, which is incorrect. I tried to account for that, but perhaps I missed. > > @@ -196,6 +197,11 @@ struct inode *hfs_new_inode(struct inode > > HFS_I(inode)->cached_blocks = 0; > > memset(HFS_I(inode)->first_extents, 0, sizeof(hfs_extent_rec)); > > memset(HFS_I(inode)->cached_extents, 0, sizeof(hfs_extent_rec)); > > + inode->i_mode = (mode & ~0777) | (~hsb->s_file_umask & 0777); > > + if (mode & S_IWUSR) > > + inode->i_mode |= S_IWUGO; > > + else > > + inode->i_mode &= ~S_IWUGO; > > } > > insert_inode_hash(inode); > > mark_inode_dirty(inode); > > Thanks, for reminding me to fix hfs_new_inode here, but the above applies. No problem. > > ===== fs/hfs/super.c 1.32 vs edited ===== > > --- 1.32/fs/hfs/super.c 2004-10-26 05:06:47 +09:00 > > +++ edited/fs/hfs/super.c 2004-11-01 20:01:54 +09:00 > > @@ -149,8 +149,8 @@ static int parse_options(char *options, > > /* initialize the sb with defaults */ > > hsb->s_uid = current->uid; > > hsb->s_gid = current->gid; > > - hsb->s_file_umask = 0644; > > - hsb->s_dir_umask = 0755; > > + hsb->s_file_umask = 0111; > > + hsb->s_dir_umask = 0000; > > hsb->s_type = hsb->s_creator = cpu_to_be32(0x3f3f3f3f); /* == '????' */ > > hsb->s_quiet = 0; > > hsb->part = -1; > > This may be closer to the mac default, where everyone can access anything, > but I'd rather keep a safe default. > Below is my updated patch. Thanks, I will give it a spin and get back to you. -- Horms ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: chmod messes up permissions on hfs filesystem 2004-11-04 3:31 ` Horms @ 2004-11-04 4:35 ` Horms 0 siblings, 0 replies; 7+ messages in thread From: Horms @ 2004-11-04 4:35 UTC (permalink / raw) To: Roman Zippel; +Cc: LKML, Siep Kroonenberg, 278068 On Thu, Nov 04, 2004 at 12:31:31PM +0900, Horms wrote: > On Wed, Nov 03, 2004 at 05:00:35PM +0100, Roman Zippel wrote: > > Hi, > > > > On Tue, 2 Nov 2004, Horms wrote: > > > > > Thanks for the patch, though the behaviour of the umask still seems > > > rather odd. I would like to offer an updated patch which I believe > > > makes the umask behave in the expected way. It also ensures > > > that the write_lock bit is read from/written to disk correctly. > > > > You apply the umask before updating the write bit, which is incorrect. > > I tried to account for that, but perhaps I missed. Hi, I think that I am a little confused here. By applying the umask after any write bits set from disk or by the user they are overridden. I thought the intention of the umask was to provide a default, not to override user-derived values. In the case of msdos, which I believe is where this came from there are no user or disk inputs as the file system does not have any permissions as such. So the umask can just be unilaterally applied. But in the case of hfs, where there is one permission bit, I think it would make sense for user requests, which are subsequently written to disk, to override the umask. Does that make sense? -- Horms ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-11-04 4:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-11-01 4:35 chmod messes up permissions on hfs filesystem Horms 2004-11-01 11:05 ` Horms 2004-11-01 16:27 ` Roman Zippel 2004-11-02 3:56 ` Horms 2004-11-03 16:00 ` Roman Zippel 2004-11-04 3:31 ` Horms 2004-11-04 4:35 ` Horms
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®