Bug 14101 - smbc_stat() doesn't return the correct st_mode and also the uid/gid is not filled (SMBv1)
smbc_stat() doesn't return the correct st_mode and also the uid/gid is not fi...
Status: REOPENED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient
unspecified
All All
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-08-23 12:15 UTC by Andreas Schneider
Modified: 2020-03-17 20:01 UTC (History)
2 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
asn: review? (jra)
Details
additional patch for 4.11 (2.86 KB, patch)
2020-03-04 11:05 UTC, Andreas Schneider
asn: review? (jra)
Details
additional patch for 4.10 (2.86 KB, patch)
2020-03-04 11:06 UTC, Andreas Schneider
asn: review? (jra)
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!