Bug 14808 - smbc_getxattr() return value is incorrect
Summary: smbc_getxattr() return value is incorrect
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.14.6
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
Depends on:
Reported: 2021-08-23 07:39 UTC by Dmitry Degtyarev
Modified: 2023-10-05 14:15 UTC (History)
6 users (show)

See Also:

git-am fix for 4.17.next, 4.16.next. (5.51 KB, patch)
2022-11-01 19:26 UTC, Jeremy Allison
jra: review? (dmulder)
slow: review+
patch for 4.18 (3.73 KB, patch)
2023-09-05 15:53 UTC, Andreas Schneider
jra: review+
slow: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Degtyarev 2021-08-23 07:39:36 UTC
Expected behaviour: "@return 0 on success, < 0 on error" (from function comment).
Actual behaviour: returns positive value on success, < 0 on error.

In the code you can see that getxattr() uses cacl_get():
And then forwards the return value from cacl_get():
Meanwhile, cacl_get() returns a non-zero value called "n_used":

(@Nable80 suggested the bugreport)
Comment 1 Samba QA Contact 2022-11-01 18:32:04 UTC
This bug was referenced in samba master:

Comment 2 Jeremy Allison 2022-11-01 19:26:34 UTC
Created attachment 17610 [details]
git-am fix for 4.17.next, 4.16.next.

Cherry-picked cleanly from master.
Comment 3 Jeremy Allison 2023-01-13 17:59:25 UTC
Comment on attachment 17610 [details]
git-am fix for 4.17.next, 4.16.next.

Ping David - can we get this into the next release please ?
Comment 4 Ralph Böhme 2023-01-13 18:27:23 UTC
Reassigning to Jule for inclusion in 4.16 and 4.17.
Comment 5 Jule Anger 2023-01-16 09:05:09 UTC
Pushed to autobuild-v4-{17,16}-test.
Comment 6 Samba QA Contact 2023-01-16 10:48:27 UTC
This bug was referenced in samba v4-16-test:

Comment 7 Samba QA Contact 2023-01-16 10:50:03 UTC
This bug was referenced in samba v4-17-test:

Comment 8 Jule Anger 2023-01-16 11:54:42 UTC
Closing out bug report.

Comment 9 Samba QA Contact 2023-01-26 17:49:49 UTC
This bug was referenced in samba v4-17-stable (Release samba-4.17.5):

Comment 10 Samba QA Contact 2023-02-16 16:36:37 UTC
This bug was referenced in samba v4-16-stable (Release samba-4.16.9):

Comment 11 Volker Lendecke 2023-04-04 10:24:56 UTC
See https://gitlab.com/samba-team/samba/-/merge_requests/3024
Comment 12 Remi Collet 2023-04-04 12:37:08 UTC
The fix in 4.16.9/4.17.5 for this issue introduces a BC break

This was really a documentation issue for the return value

Documentation for size parameter describes the usage
of the returned needed size:

 * @param size      The size of the buffer pointed to by value.  This parameter
 *                  may also be zero, in which case the size of the buffer
 *                  required to hold the attribute value will be returned,
 *                  but nothing will be placed into the value buffer.

Which is also consistent with getxattr syscall:

  On success, these calls return a nonnegative value which is the size (in bytes) of the extended attribute value.
  On failure, -1 is re-turned and errno is set to indicate the error.

In short, the documentation needs to be fixed, not the function behavior (especially in a minor release)
Comment 13 Dmitry Degtyarev 2023-04-04 15:29:00 UTC
Didn't realize there's code that depends on getxattr() returning size. Reverting the fix and changing the documentation instead is definitely the way to go then.
Comment 14 Jeremy Allison 2023-04-04 16:11:28 UTC
Fix looks correct. RB+ by me on the MR. Just needs a second Team reviewer. Thanks for catching this.
Comment 15 Samba QA Contact 2023-04-09 10:45:04 UTC
This bug was referenced in samba master:

Comment 16 Remi Collet 2023-04-11 09:53:18 UTC
Please also apply in 4.16, 4.17, 4.18 where the regression appears
Comment 17 Andreas Schneider 2023-09-05 15:53:00 UTC
This is fixed in 4.19 and 4.17, but NOT in 4.18!
Comment 18 Andreas Schneider 2023-09-05 15:53:25 UTC
Created attachment 18078 [details]
patch for 4.18
Comment 19 Jeremy Allison 2023-09-05 17:34:51 UTC
Re-assigning to Jule to update the 4.18.next release.
Comment 20 Jule Anger 2023-09-06 08:35:40 UTC
Pushed to autobuild-v4-18-test.
Comment 21 Samba QA Contact 2023-09-06 09:29:05 UTC
This bug was referenced in samba v4-18-test:

Comment 22 Jule Anger 2023-09-06 09:58:32 UTC
Closing out bug report.

Comment 23 Samba QA Contact 2023-09-27 08:14:48 UTC
This bug was referenced in samba v4-18-stable (Release samba-4.18.7):

Comment 24 Remi Collet 2023-10-05 14:15:07 UTC
Notice that 4.18.7 and 4.19 are OK
but 4.17.11 is still broken