| Message ID | 1283848562-19564-1-git-send-email-dedekind1@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2010-09-07 at 11:36 +0300, Artem Bityutskiy wrote: > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > When mkfs.ubifs is used with -r dir, it does not make the root UBIFS > inode uid/gid/permissions to be equivalent to dir's permissions, but > it makes root inode permissions to be equivalent to uid = git = 0 > (root) and permissions = u+rwx go+rx. > > This patch changes the behavior and makes mkfs.ubifs use the > permissions of the directory containing the original files on the host. > I.e., it will be <dir>'s uid/git/permissions if case of mkfs.ubifs > -r <dir>. > > This patch is a bit dangerous because it changes the behavior and may > have security implications if someone used the older version, relied > on this bug, and upgrades to the newer version. > > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> All mkfs.ubifs users should take a look at this - should we apply this patch? I'm still in doubt...
Artem Bityutskiy wrote: > On Tue, 2010-09-07 at 11:36 +0300, Artem Bityutskiy wrote: >> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> >> >> When mkfs.ubifs is used with -r dir, it does not make the root UBIFS >> inode uid/gid/permissions to be equivalent to dir's permissions, but >> it makes root inode permissions to be equivalent to uid = git = 0 >> (root) and permissions = u+rwx go+rx. >> >> This patch changes the behavior and makes mkfs.ubifs use the >> permissions of the directory containing the original files on the host. >> I.e., it will be <dir>'s uid/git/permissions if case of mkfs.ubifs >> -r <dir>. >> >> This patch is a bit dangerous because it changes the behavior and may >> have security implications if someone used the older version, relied >> on this bug, and upgrades to the newer version. >> >> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > All mkfs.ubifs users should take a look at this - should we apply this > patch? I'm still in doubt... > I do not agree with changing the behaviour. It should be a new option, and you could add a warning explaining what the root inode permissions are and why e.g. Warning: Option ?? not used. Setting root inode permissions to blah Warning: Option ?? used. Setting root inode permissions to blah
On Tue, 2010-09-07 at 13:23 +0300, Adrian Hunter wrote: > Artem Bityutskiy wrote: > > On Tue, 2010-09-07 at 11:36 +0300, Artem Bityutskiy wrote: > >> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > >> > >> When mkfs.ubifs is used with -r dir, it does not make the root UBIFS > >> inode uid/gid/permissions to be equivalent to dir's permissions, but > >> it makes root inode permissions to be equivalent to uid = git = 0 > >> (root) and permissions = u+rwx go+rx. > >> > >> This patch changes the behavior and makes mkfs.ubifs use the > >> permissions of the directory containing the original files on the host. > >> I.e., it will be <dir>'s uid/git/permissions if case of mkfs.ubifs > >> -r <dir>. > >> > >> This patch is a bit dangerous because it changes the behavior and may > >> have security implications if someone used the older version, relied > >> on this bug, and upgrades to the newer version. > >> > >> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > > > All mkfs.ubifs users should take a look at this - should we apply this > > patch? I'm still in doubt... > > > > I do not agree with changing the behaviour. It should be a new option, > and you could add a warning explaining what the root inode permissions > are and why e.g. But on the other hand, a separate option looks silly... Would be nice to somehow slowly deprecate current behavior... > Warning: Option ?? not used. Setting root inode permissions to blah > > Warning: Option ?? used. Setting root inode permissions to blah
On Tue, 2010-09-07 at 13:23 +0300, Adrian Hunter wrote: > I do not agree with changing the behaviour. It should be a new option, > and you could add a warning explaining what the root inode permissions > are and why e.g. > > Warning: Option ?? not used. Setting root inode permissions to blah > > Warning: Option ?? used. Setting root inode permissions to blah Sent you v2 of this patch with an alternative (less harsh) solution.
diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c index 9f2a226..a4aebcb 100644 --- a/mkfs.ubifs/mkfs.ubifs.c +++ b/mkfs.ubifs/mkfs.ubifs.c @@ -1639,9 +1639,8 @@ static int write_data(void) } else { root_st.st_mtime = time(NULL); root_st.st_atime = root_st.st_ctime = root_st.st_mtime; + root_st.st_mode = S_IFDIR | S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; } - root_st.st_uid = root_st.st_gid = 0; - root_st.st_mode = S_IFDIR | S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; head_flags = 0; err = add_directory(root, UBIFS_ROOT_INO, &root_st, !root);