Bug 9317

Summary: pseudo: incorrect xattr semantic for hardlinks
Product: [Yocto Project Subprojects] Pseudo Reporter: Patrick Ohly <patrick.ohly>
Component: pseudoAssignee: Joshua Lock <joshuagloe>
Status: RESOLVED FIXED QA Contact: David Rodriguez <david.israelx.rodriguez.castellanos>
Severity: normal    
Priority: Medium+ CC: jose.perez.carranza, joshuagloe, meta.mr.watcher, meta.watcher, randy.macleod, seebs, sgw, yp.pseudo.watcher, yp.watcher
Version: master   
Target Milestone: 2.2 M2   
Hardware: x86   
OS: Multiple   
Whiteboard:
OS type for building Yocto: --- Type of Regression: ---
Verified: Documentation change: No (bug/feature does not impact docs)
Bug Depends on:    
Bug Blocks: 9858, 9857    

Description Patrick Ohly 2016-03-21 19:39:48 UTC 
The correct semantic for hardlinks is that xattrs are associated with the inode. Under pseudo, they get associated incorrectly with the the directory entry.

The effect is that operations are possible under pseudo which aren't possible in the final file system (which is confusing for developers) and (more important) that copying entire filesystems with "tar" fails to copy hardlinks correctly. Apparently tar only sets the xattrs once, leading to filesystem entries under pseudo where one copy has the expected attributes and the other doesn't.

Expected behavior (from a Linux host):

# echo hello >foo
# setfattr -n security.SMACK64 -v System foo
# ln foo bar
# getfattr -d -m . foo bar
# file: foo
security.SMACK64="System"

# file: bar
security.SMACK64="System"

Incorrect behavior under pseudo:

# echo hello >foo
# setfattr -n security.SMACK64 -v System foo
root@pohly-desktop:/tmp# ln foo bar
root@pohly-desktop:/tmp# getfattr -d -m . foo bar
# file: foo
security.SMACK64="System"

Nothing gets reported for bar.

We ran into this as part of the integration of meta-swupd into Ostro OS. We can work around it for now by avoiding hardlinks, but long term it would be better to get pseudo fixed.
Comment 1 Seebs 2016-03-21 20:53:11 UTC 
Huh. That's a good point. I suspect pseudo currently has some other issues to do with hard links ending up with the wrong modes, and it's not totally surprising to see it happening also with extended attributes.

So, yeah, this should be fixed. I'll start thinking about it.
Comment 2 Patrick Ohly 2016-03-21 22:00:51 UTC 
I remember that at some point I accidentally set mode bits differently on hardlinks and that it worked under pseudo, but then of course not in the real filesystem. But when I tested that again today, it worked as expected under pseudo (modes where the same on all copies).
Comment 3 Seebs 2016-03-22 19:11:41 UTC 
I'm not sure whether I have anything in the code to intentionally fix hardlinks, but I might. I'll look at that.

Mostly, I need to make the xattr stuff smarter, in that it needs to look for xattrs on anything with the same inode as the requested path. Eww. This is gonna be sorta tricky and I have no immediate ETA.
Comment 4 Seebs 2016-03-23 20:06:52 UTC 
So, a quick overview of why this is busted, and possibly hard to fix:

The files database has device, inode, and path. The xattr database uses row_id and path, with the row id being the row id of the specific entry in the files database which the xattr was created for.

When doing updates to the regular file database, I do actually change everything with the same device and inode. But for xattrs, I'm just doing the one row. That's probably an error. There was, at the time, some perfectly good technical reason for which I wanted to use the row ID as the key, rather than the device/inode, although I don't remember my reasoning now. But that means that I need to do something significantly fancier to get xattrs to work correctly.

... actually, what I'll probably do is create/change entries for everything with the same device/inode. I don't want to change the table structure now. Ugh.
Comment 5 Seebs 2016-03-25 21:05:02 UTC 
Okay, I have proposed fixes for this in my tree, they seem to work, but it's gonna take some time to get code review etc.
Comment 6 Seebs 2016-04-29 19:59:18 UTC 
Just for the record, this is probably-done, but I haven't gotten it code-reviewed yet.
Comment 7 Joshua Lock 2016-07-07 09:17:43 UTC 
We have a recipe which upgrades to the latest ~1.8 pseudo release and fixes some of the issues identified within. This recipe and related changes are currently undergoing test and review.
Comment 8 Joshua Lock 2016-07-12 16:17:18 UTC 
This issue was fixed in the 1.8 branch of pseudo, we're now using 1.8.1 in the master branches of openembedded-core and poky.