- 
                Notifications
    
You must be signed in to change notification settings  - Fork 36
 
Explicitly avoid trying to remove xattrs on symlink #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e5f0218    to
    22f0613      
    Compare
  
    
           | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
| 
           FWIW with mainline kernel in ubuntu jammy I don't get these stray xattrs. With 5.15.0-53-generic kernel in ubuntu jammy, I do.  | 
    
          
 it would be nice to file an ubuntu bug if you can. That’d also give sobering to point at for a fix.  | 
    
You cannot have user.* xattrs on a symlink.
Except...  overlay is in the kernel, it can do what it wants.
Unfortunately, fs/xattr.c:xattr_permission() won't allow us
to do anything with them:
 124         /*
 125          * In the user.* namespace, only regular files and directories can have
 126          * extended attributes. For sticky directories, only the owner and
 127          * privileged users can write attributes.
 128          */
 129         if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
 130                 if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
 131                         return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
So just skip trying to remove any xattrs on symlinks.
This *should* be ok, because if I do mksquashfs of a directory
with a symlink with user.overlay.origin xattr and then use squashfuse
to mount it, the xattr is no longer there.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
    22f0613    to
    1a0f2cb      
    Compare
  
    
          
 I can give pretty simple reproduction instructions using stacker, but it'd be nicer if I could do so with just a manual overlay example.  | 
    
          
 "sobering". Thanks iphone. That should have said: That'd also give us something to point at for a fix.  | 
    
| 
           It took me a few minutes but I did decipher it as 'something' :-)  | 
    
| 
           I've failed to reproduce this by hand so far. But using stacker, it's just this stacker.yaml: which ends with This is reproducible on 5.15.0-53-generic and 5.19.0-21-generic, but not on the ubuntu mainline build (6.1.0-060100rc5-generic)  | 
    
| 
           Maybe instead of simply ignoring all errors, we should try to read the contents. If that returns ENODATA or EPERM then don't try to remove it. But it's more work for probably very little gain.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
You cannot have user.* xattrs on a symlink.
Except... overlay is in the kernel, it can do what it wants. Unfortunately, fs/xattr.c:xattr_permission() won't allow us to do anything with them:
124 /*
125 * In the user.* namespace, only regular files and directories can have
126 * extended attributes. For sticky directories, only the owner and
127 * privileged users can write attributes.
128 */
129 if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
130 if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
131 return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
So just skip trying to remove any xattrs on symlinks.
This should be ok, because if I do mksquashfs of a directory with a symlink with user.overlay.origin xattr and then use squashfuse to mount it, the xattr is no longer there.
Signed-off-by: Serge Hallyn serge@hallyn.com