Bug 14101 - smbc_stat() doesn't return the correct st_mode and also the uid/gid is not filled (SMBv1)
Summary: smbc_stat() doesn't return the correct st_mode and also the uid/gid is not fi...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 14379 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-08-23 12:15 UTC by Andreas Schneider
Modified: 2020-05-15 09:58 UTC (History)
5 users (show)

See Also:


Attachments
patch for 4.11 (28.41 KB, patch)
2019-12-19 16:00 UTC, Andreas Schneider
jra: review+
Details
patch for 4.10 (28.41 KB, patch)
2019-12-19 16:00 UTC, Andreas Schneider
jra: review+
Details
additional patch for 4.12 (2.86 KB, patch)
2020-03-04 11:04 UTC, Andreas Schneider
no flags Details
additional patch for 4.11 (2.86 KB, patch)
2020-03-04 11:05 UTC, Andreas Schneider
no flags Details
additional patch for 4.10 (2.86 KB, patch)
2020-03-04 11:06 UTC, Andreas Schneider
no flags Details
patch to remove it again for 4.12 and 4.11 and 4.10. (2.95 KB, patch)
2020-04-08 09:31 UTC, Andreas Schneider
ab: review+
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2019-08-23 12:15:46 UTC
Create a share with unix extensions enabled and try to stat it.
Comment 2 Andreas Schneider 2019-12-19 16:00:12 UTC
Created attachment 15712 [details]
patch for 4.11
Comment 3 Andreas Schneider 2019-12-19 16:00:53 UTC
Created attachment 15714 [details]
patch for 4.10
Comment 4 Jeremy Allison 2019-12-19 20:58:24 UTC
Re-assigning to Karolin for inclusion in 4.11.next, 4.10.next
Comment 5 Karolin Seeger 2020-01-14 08:07:45 UTC
(In reply to Jeremy Allison from comment #4)
Pushed to autobuild-v4-{11,10}-test.
Comment 6 Karolin Seeger 2020-01-15 11:10:56 UTC
(In reply to Karolin Seeger from comment #5)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 7 Volker Lendecke 2020-02-25 17:41:13 UTC
See https://gitlab.com/samba-team/devel/samba/pipelines/120957076
Comment 8 Karolin Seeger 2020-02-26 08:10:13 UTC
(In reply to Volker Lendecke from comment #7)
Is it 4.10 and 4.11 only (applies cleanly on 4.12)?
And is the first patchset for 4.12, also?
Comment 9 Andreas Schneider 2020-03-04 11:04:06 UTC
Created attachment 15839 [details]
additional patch for 4.12
Comment 10 Andreas Schneider 2020-03-04 11:05:18 UTC
Created attachment 15840 [details]
additional patch for 4.11
Comment 11 Andreas Schneider 2020-03-04 11:06:14 UTC
Created attachment 15841 [details]
additional patch for 4.10
Comment 12 Jeremy Allison 2020-03-06 18:05:38 UTC
Andreas - are these extra patches complete ? Volker did lots more work inside libsmbclient in master to fix the tests and make readdirplusXXX use the POSIX extensions.

Do we need those changes also ?
Comment 13 Andreas Schneider 2020-03-09 06:19:39 UTC
I've just backported what was part of that Merge Request. I didn't look into the tests. I think we could backport the tests to 4.12 if you want but for lower version there are probably too many changes.
Comment 14 Jeremy Allison 2020-03-09 18:40:37 UTC
It's not the tests that actually matter, it's the code that changes cli_list() to use the POSIX extensions etc.

Look at:

git format-patch --stdout 9cc546956dcfdf7c898154aa79d0947eb4c9c578..a1856f07b39d4e8bc8f66cb49c7666b0d5ed99b8

I think that's the bare minimum on top of what you already have. Adding the tests as well will make sure you got it right.
Comment 15 Jeremy Allison 2020-03-10 00:03:42 UTC
Actually, don't merge those fixes yet. I have one thing to check with Volker first...
Comment 16 Volker Lendecke 2020-03-16 20:23:15 UTC
Jeremy, Andreas, any updates here? The code in master has a valgrind error. I've tried to fix this in 51551e0d53f, but this lead to an avalance of more and more posix functionality in libsmb, which eventually led to 9653a107389, re-introducing the original valgrind error.
Comment 17 Jeremy Allison 2020-03-16 20:45:36 UTC
Nope, working on other stuff. I had a call with Andreas, where we decided we needed an explicit smbc_I_want_posix() sort of call, but until then probably the best solution is just never set the try_posixinfo variable.
Comment 18 Volker Lendecke 2020-03-16 20:50:05 UTC
Until we have that call, I think we need a different solution for the valgrind error. Any idea?
Comment 19 Volker Lendecke 2020-03-16 20:50:57 UTC
What I mean: I think we should not just leave this "frame" vs "fname" error in the code, this needs fixing.
Comment 20 Jeremy Allison 2020-03-16 20:52:41 UTC
Sure, but why not just fix and comment this out as it should be falling back anyway ?
Comment 21 Volker Lendecke 2020-03-17 06:49:00 UTC
If I see it correctly, this would basically mean the patchset for this bug won't be used at all. But that means that this bug won't be fixed properly.

Andreas, what do you think?
Comment 22 Andreas Schneider 2020-03-17 06:58:49 UTC
Lets comment out the code to disable try_posixinfo till we have a proper solution.
Comment 23 Andreas Schneider 2020-03-17 06:59:29 UTC
Sorry for the inconvenience here.
Comment 24 Volker Lendecke 2020-03-17 20:01:22 UTC
(In reply to Andreas Schneider from comment #22)
> Lets comment out the code to disable try_posixinfo till we have a proper
> solution.

Can you explain a bit more what you want to say here? You want us to never disable try_posixinfo? This would mean that with every stat we run into the valgrind error. Do you have time to give me a quick hint or provide a simple patch?

Thanks a *LOT* for your cooperation!
Comment 25 Andreas Schneider 2020-04-08 09:31:59 UTC
Created attachment 15901 [details]
patch to remove it again for 4.12 and 4.11 and 4.10.
Comment 26 Alexander Bokovoy 2020-04-11 20:13:24 UTC
Comment on attachment 15901 [details]
patch to remove it again for 4.12 and 4.11 and 4.10.

I provided a test build to Fedora users and that solved a long-standing issue reported as https://bugzilla.redhat.com/show_bug.cgi?id=1801442#c28 which people already started reproducing in other distributions.
Comment 27 Alexander Bokovoy 2020-04-27 08:12:31 UTC
A revert was pushed out to Fedora stable updates yesterday:

F32: https://bodhi.fedoraproject.org/updates/FEDORA-2020-7f0df2c653
F31: https://bodhi.fedoraproject.org/updates/FEDORA-2020-2655ce257f

For the record, without this revert people were seeing some files shown as folders when talking to SMBv1 servers. This affects many Linux distributions already.
Comment 28 Andreas Schneider 2020-04-27 08:14:35 UTC
Karolin, please apply the patch to remove it please. Sorry for the inconvenience ... Thanks.
Comment 29 Jeremy Allison 2020-04-27 16:43:42 UTC
Comment on attachment 15901 [details]
patch to remove it again for 4.12 and 4.11 and 4.10.

Sorry for the delay.
Comment 30 Karolin Seeger 2020-05-04 09:08:24 UTC
(In reply to Jeremy Allison from comment #29)
Pushed to autobuild-v4-{12,11,10}-test.
Comment 31 Karolin Seeger 2020-05-05 06:32:53 UTC
(In reply to Karolin Seeger from comment #30)
Pushed to all branches.
Closing out bug report.

Thanks!
Comment 32 Harald Sitter 2020-05-15 09:58:32 UTC
*** Bug 14379 has been marked as a duplicate of this bug. ***