Dr. Thomas Orgis
2022-Jan-27 20:30 UTC
[Samba] Patch for bug 14727 / samba > 4.13 in pkgsrc
Hi,
since access to Samba's bugzilla seems to be manually curated and I'm
only a very casual contributor, I hope it's OK for me to discuss
https://bugzilla.samba.org/show_bug.cgi?id=14727
a bit here. In the process of updating samba4 in pkgsrc from 4.13, I'm
intending to follow this suggestion:
Ralph B?hme 2021-06-03 11:48:39 UTC
(In reply to Jeremy Allison from comment #3)
What if we just filter out the O_CREAT in this case? We know we're
opening an existing file so we don't need to pass O_CREAT. Easy fix
for the BSDs and not really a big heritage to carry along.
Is this the correct change for this?
--- source3/smbd/open.c.orig 2022-01-27 18:20:00.791110968 +0000
+++ source3/smbd/open.c
@@ -1189,11 +1189,13 @@ static NTSTATUS reopen_from_procfd(struc
fsp->fsp_flags.is_pathref = false;
+ /* O_CREAT not useful for reopening, and it upsets BSD kernels,
+ * see https://bugzilla.samba.org/show_bug.cgi?id=14727 . */
new_fd = SMB_VFS_OPENAT(fsp->conn,
fsp->conn->cwd_fsp,
&proc_fname,
fsp,
- flags,
+ flags & ~O_CREAT,
mode);
if (new_fd == -1) {
status = map_nt_error_from_unix(errno);
I am not sure about where the intial open occurs for the 'reopen' here.
But it is the point that we do have a descriptor for the file described
by fsp already and want to open another one, right ? and for some
reason via /proc/?
Is the discussed change that simple and harmless? Maybe harmless enough
to just include it and close the bug?
Alrighty then,
Thomas
PS:
On updating the samba4 package in pkgsrc, I do notice quite a number of
portability patches in there. A notable change is a set of
+ memset(&tp->mutex, 0, sizeof(pthread_mutex_t));
ret = pthread_mutex_init(&tp->mutex, NULL);
with that reason:
On SunOS (OpenSolaris), pthread_mutex_init() expects a zeroed-out
mutex data structure
I'm not sure how current that behaviour is and if one really wants to
continue to cater for this speciality.
Hm, and I see something about primary groups for winbind_nss_netbsd ?
and various hacks for the build system which might be special to the
pkgsrc install method. I'll repeat the winbind one here:
$NetBSD: patch-nsswitch_winbind__nss__netbsd.c,v 1.2 2020/07/21 18:42:25
christos Exp $
Syntax error fix.
Add primary group support
--- nsswitch/winbind_nss_netbsd.c.orig 2020-01-31 05:25:36.000000000 -0500
+++ nsswitch/winbind_nss_netbsd.c 2020-07-21 10:24:19.651265315 -0400
@@ -176,6 +176,7 @@
{
int *result = va_arg(ap, int *);
const char *uname = va_arg(ap, const char *);
+ gid_t agroup = va_arg(ap, gid_t);
gid_t *groups = va_arg(ap, gid_t *);
int maxgrp = va_arg(ap, int);
int *groupc = va_arg(ap, int *);
@@ -185,10 +186,17 @@
};
struct winbindd_response response = {
.length = 0,
- }
+ };
gid_t *wblistv;
int wblistc, i, isdup, dupc;
+ /* add the primary group */
+ if (*groupc < maxgrp)
+ groups[*groupc] = agroup;
+ else
+ *result = -1;
+ (*groupc)++;
+
strncpy(request.data.username, uname,
sizeof(request.data.username) - 1);
i = winbindd_request_response(NULL, WINBINDD_GETGROUPS,
Does it make sense to push that upstream to Samba? I also see some
rather trivial header portability fixes. You can check those on
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/net/samba4/patches/
--
Dr. Thomas Orgis
HPC @ Universit?t Hamburg
Andrew Bartlett
2022-Jan-27 20:52 UTC
[Samba] Patch for bug 14727 / samba > 4.13 in pkgsrc
On Thu, 2022-01-27 at 21:30 +0100, Dr. Thomas Orgis via samba wrote:> Hi, > > since access to Samba's bugzilla seems to be manually curated and I'm > only a very casual contributor, I hope it's OK for me to discuss > > https://bugzilla.samba.org/show_bug.cgi?id=14727 > > a bit here. In the process of updating samba4 in pkgsrc from 4.13,The Samba and (perhaps more on-topic) samba-technical mailing lists are a valid forum, but you are most welcome in our bugzilla, I've sent you an invite. The barrier to entry to due to spam.> PS: > > On updating the samba4 package in pkgsrc, I do notice quite a number > of > portability patches in there. A notable change is a set ofWe would strongly prefer if patches carried in distributors were first proposed upstream. Samba in Debian for example has a fairly strong rule that local patches are not allowed except after they have been put into Samba master, or if they are truly Debian-specific. This helps us in upstream vet the patches and avoids the maintenance cost because they are autocratically included in new releases. (Historically some patches included, and certainly a number that were proposed for 'portability' in Samba packages would not have been accepted upstream, and would have caused issues).> + memset(&tp->mutex, 0, sizeof(pthread_mutex_t)); > ret = pthread_mutex_init(&tp->mutex, NULL); > > with that reason: > > On SunOS (OpenSolaris), pthread_mutex_init() expects a zeroed- > out > mutex data structure > > I'm not sure how current that behaviour is and if one really wants to > continue to cater for this speciality. > > Hm, and I see something about primary groups for winbind_nss_netbsd ? > and various hacks for the build system which might be special to the > pkgsrc install method. I'll repeat the winbind one here: > > $NetBSD: patch-nsswitch_winbind__nss__netbsd.c,v 1.2 2020/07/21 > 18:42:25 christos Exp $ > > Syntax error fix. > Add primary group support > > --- nsswitch/winbind_nss_netbsd.c.orig 2020-01-31 > 05:25:36.000000000 -0500 > +++ nsswitch/winbind_nss_netbsd.c 2020-07-21 10:24:19.651265315 > -0400 > @@ -176,6 +176,7 @@ > { > int *result = va_arg(ap, int *); > const char *uname = va_arg(ap, const char *); > + gid_t agroup = va_arg(ap, gid_t); > gid_t *groups = va_arg(ap, gid_t *); > int maxgrp = va_arg(ap, int); > int *groupc = va_arg(ap, int *); > @@ -185,10 +186,17 @@ > }; > struct winbindd_response response = { > .length = 0, > - } > + }; > gid_t *wblistv; > int wblistc, i, isdup, dupc; > > + /* add the primary group */ > + if (*groupc < maxgrp) > + groups[*groupc] = agroup; > + else > + *result = -1; > + (*groupc)++; > + > strncpy(request.data.username, uname, > sizeof(request.data.username) - 1); > i = winbindd_request_response(NULL, WINBINDD_GETGROUPS, > > > Does it make sense to push that upstream to Samba? I also see some > rather trivial header portability fixes. You can check those on > > http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/net/samba4/patches/All packager patches should be proposed upstream for comment in my view. We have seen similar things with FreeBSD, you are not alone. Andrew Bartlett -- Andrew Bartlett (he/him) https://samba.org/~abartlet/ Samba Team Member (since 2001) https://samba.org Samba Team Lead, Catalyst IT https://catalyst.net.nz/services/samba Samba Development and Support, Catalyst IT - Expert Open Source Solutions
On 1/27/22 21:30, Dr. Thomas Orgis via samba wrote:> since access to Samba's bugzilla seems to be manually curated and I'm > only a very casual contributor, I hope it's OK for me to discuss > > https://bugzilla.samba.org/show_bug.cgi?id=14727 > > a bit here. In the process of updating samba4 in pkgsrc from 4.13, I'm > intending to follow this suggestion: > > Ralph B?hme 2021-06-03 11:48:39 UTC > > (In reply to Jeremy Allison from comment #3) > What if we just filter out the O_CREAT in this case? We know we're > opening an existing file so we don't need to pass O_CREAT. Easy fix > for the BSDs and not really a big heritage to carry along. > > Is this the correct change for this? > > --- source3/smbd/open.c.orig 2022-01-27 18:20:00.791110968 +0000 > +++ source3/smbd/open.c > @@ -1189,11 +1189,13 @@ static NTSTATUS reopen_from_procfd(struc > > fsp->fsp_flags.is_pathref = false; > > + /* O_CREAT not useful for reopening, and it upsets BSD kernels, > + * see https://bugzilla.samba.org/show_bug.cgi?id=14727 . */ > new_fd = SMB_VFS_OPENAT(fsp->conn, > fsp->conn->cwd_fsp, > &proc_fname, > fsp, > - flags, > + flags & ~O_CREAT, > mode); > if (new_fd == -1) { > status = map_nt_error_from_unix(errno);Andrew, is this a problem on FreeBSD as well and if so, how do you deal with this? -slow -- Ralph Boehme, Samba Team https://samba.org/ SerNet Samba Team Lead https://sernet.de/en/team-samba -------------- next part -------------- A non-text attachment was scrubbed... Name: OpenPGP_signature Type: application/pgp-signature Size: 840 bytes Desc: OpenPGP digital signature URL: <http://lists.samba.org/pipermail/samba/attachments/20220128/faf8c3b4/OpenPGP_signature.sig>