On 10/26/17 5:50 PM, Luis R. Rodriguez wrote: > symlinks with the attribute append set cannot be removed as they are > treated as if a file was set with the immutable flag set. Standard > tools however cannot remove *any* attribute flag: > > chattr -a symlink > chattr: Operation not supported while reading flags on b > > Furthermore one cannot list attributes on symlinks using lsattr either: > > # touch a > # ln -s a b > # lsattr b > lsattr: Operation not supported While reading flags on b ok, so tools in e2fsprogs won't clear/set... which is one datapoint... > This means that if you end up with an append attribute on a symlink > you need special tools to try to remove it. This begs the question of > whether or not XFS should also disallow attributes on symlinks. http://begthequestion.info/ ;) > I can trace the lsattr change back down to commit 023d111e92195 > ("chattr.1.in: Document the compression attribute flags E, X, and ...") > on e2fsprogs since August 2002. That commit just refers to the fact > that attributes were only allowed after that commit for directories > and regular files due to Debian bug 152029 [0]. This bug was filed since > lsattr used to enable attributes even on special files and lsattr would > segfault when used against them since when used against some old DRM > buggy driver the ioctl seem to have worked bug crashed lsattr. The bug > report doesn't list any specific reasoning for not allowing attributes > on symlinks though. Super. > Although in XFS you could end up in a situation where attributes > are set on a symlink this is ather hard to do, likewie for clearing > them. > > Its unclear if XFS purposely supports attributes on symlinks or > if this was unintentional. > > The ways in which you can end up to setting / unsetting / getting attributes > on symlinks on XFS are: > > a) corruption > b) xfsctl() > c) xfs_db [1] [2] - only when umounted > > POSIX doesn't seem to actually have a position on this. > > These are not extended attributes, however xattr(7) does have > a little nugget worth considering regarding this, and for which > it was decided to at least not support extended attributes on > symlinks or special files: > > The file permissions of symbolic links are not used in > access checks. These differences would allow users to consume > filesystem resources in a way not controllable by disk quotas > for group or world writable special files and directories. > > For this reason, extended user attributes are allowed only for > regular files and directories. Hm, that seems like a different issue/rationale, no? > For these reasons, and since corruption is the only current valid > case where these seem to have popped up -- apply the same logic on > XFS, otherwise we'd be going against the e2fsprogs chattr convention > set since August 2002, and which folks likely are used to. > > This fixes filesystems which have these attributes set where clearly > they were set by corruption. OK but hang on, I think a core question is: Does the kernel allow us to set attributes on symlinks? If not, repair should clear them. But if it does, chattr's refusal to clear them is a quirk of that specific tool, not something that xfs_repair needs to worry about ... If the kernel provides an interface (xfsctl?) to set/clear these attributes, I don't think xfs_repair should be classifying their presence as corruption, right? If no, really, these should never be set, then the kernel (including the xfsctl route) should reject it - and at that point, I'd be ok with xfs_repair clearing them as "corruption." -Eric > [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=152029 > [1] xfs_db -x -c 'inode inode-number' -c 'write core.append 1' /dev/device > [2] xfs_db -x -c 'inode inode-number' -c 'write core.append 0' /dev/device > > Cc: "Laurent Bonnaud" <Laurent.Bonnaud@xxxxxxx> > Cc: Tso Ted <tytso@xxxxxxx> > Cc: Nikolay Borisov <nborisov@xxxxxxxx> > Cc: Flex Liu <fliu@xxxxxxxx> > Cc: Jake Norris <jake.norris@xxxxxxxx> > Cc: Jan Kara <jack@xxxxxxx> > Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx> > --- > > Otherwise if we *do* wish to support some of these we should > document the semantics. Otherwise this seems just odd and users > can end up with unexpected situations not being able to easily > clear this type of corruption. > > The append attribute is particularly annoying given you can't > remove these symlinks at all, and the first thing naturally a > user could do is try 'lsattr' or 'chattr'. Obviously this won't > help and the user will just get frustrated. > > repair/dinode.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/repair/dinode.c b/repair/dinode.c > index 15ba8cc22b39..03e93a6e17c0 100644 > --- a/repair/dinode.c > +++ b/repair/dinode.c > @@ -2482,6 +2482,41 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), > FS_XFLAG_EXTSIZE); > } > } > + if (flags & (XFS_DIFLAG_REALTIME | XFS_DIFLAG_PREALLOC | > + XFS_DIFLAG_IMMUTABLE | XFS_DIFLAG_APPEND | > + XFS_DIFLAG_SYNC | XFS_DIFLAG_NOATIME | > + XFS_DIFLAG_NODUMP | XFS_DIFLAG_RTINHERIT | > + XFS_DIFLAG_PROJINHERIT | XFS_DIFLAG_NOSYMLINKS | > + XFS_DIFLAG_EXTSIZE | XFS_DIFLAG_EXTSZINHERIT | > + XFS_DIFLAG_NODEFRAG | XFS_DIFLAG_FILESTREAM | > + XFS_DIFLAG2_DAX | XFS_DIFLAG2_COWEXTSIZE)) { > + /* chattr cannot clear append on symlink */ > + if (di_mode && S_ISLNK(di_mode) && > + !S_ISREG(di_mode) && !S_ISDIR(di_mode)) { > + if (!uncertain) { > + do_warn( > + _("file or directory attributes set on symlink inode %" PRIu64 "\n"), > + lino); > + } > + flags &= ~(XFS_DIFLAG_REALTIME | > + XFS_DIFLAG_PREALLOC | > + XFS_DIFLAG_IMMUTABLE | > + XFS_DIFLAG_APPEND | > + XFS_DIFLAG_SYNC | > + XFS_DIFLAG_NOATIME | > + XFS_DIFLAG_NODUMP | > + XFS_DIFLAG_RTINHERIT | > + XFS_DIFLAG_PROJINHERIT | > + XFS_DIFLAG_NOSYMLINKS | > + XFS_DIFLAG_EXTSIZE | > + XFS_DIFLAG_EXTSZINHERIT | > + XFS_DIFLAG_NODEFRAG | > + XFS_DIFLAG_FILESTREAM | > + XFS_DIFLAG2_DAX | > + XFS_DIFLAG2_COWEXTSIZE); > + } > + } > + > if (!verify_mode && flags != be16_to_cpu(dino->di_flags)) { > if (!no_modify) { > do_warn(_("fixing bad flags.\n")); > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
