Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
23.11.2022 00:23, ArtMG wrote: .. Attachments: * fruit-disable-useless-size_t-overflow-check.patch Thanks for the patch Michael, I compiled and ran and although it got out of the disk size functions this time, it still slipped up with I pushed this patch to debian samba package now. The check there (which this patch removes) makes no sense on 64bits (since the value is capped elsewhere) and is obviously wrong on 32bits, - for some reason it tries to calculate capacity in apples when dealing with oranges all the way, and when sizes of apples and oranges are entirely different on this platform, this test obviously doe not do any good. Also, this is a very specific issue affecting only very specific use cases (with MacOS operability on 32bit), so there should be no big harm done this way to make samba upstream unhappy (if it were for something more widespread, I'd not push this change to debian package). ../../source3/smbd/service.c:787(make_connection_snum) make_connection_snum: canonicalize_connect_path failed for service TimeMachineBackup, path /mnt/HDD/TimeMachine So this particular issue is gone now. It might have other issues, maybe due to size of size_t on 32bit in some other place, maybe due to disk size limits, maybe something entirely different, but this particular issue is gone. I do appreciate you cutting this patch for this issue, however I feel like I've been down this road before with these issues. Push a fix here, and if a new issue doesn't pop up over there straight away, give it a version or three and it will eventually. Hence my reference to whack-a-mole. It's not whack-a-mole really. Whack-a-mole assumes there's just one mole out there, which shows in different holes. But here, we have many moles collected over the years of no testing. You whack one for good, but another dozen are chewing stuff somewhere else nearby, ready to meet you. This issue already whacked two which was in the same place. As Andrew points out, this is somewhat outlying as test-cases go. There are few people wanting to rebuild a 32-bit instance of their backup mechanism for each point revision of a large and actively-developed service like samba. Well, once it's in debian, 32bit packages are provided automatically, there's no need to rebuild them ;) I see no reason, personally, to push for developer attention when the solution is as simple as 'switch to 64-bit OS', and then even OLD versions in all the repos work just fine. Although I do not have statistics to hand, I feel that we must be far enough along the migration curve on the journey from 32- to 64-bit systems, especially when it comes to a core-yet-commodity infrastructure service like samba, to warrant letting go of legacy. It is not that simple, but can be viewed like this anyway. Yes, 32bit world is dying, and the future is 64+bits, this is not question whatsoever. However, the question is *when* to declare the death of 32bit world. We either declare it dead and stop building/providing samba on any 32bit platform entirely, or we do not declare 32bit world dead, and provide a good service there. What we do have now is neither one nor another, 32bit world is not dead and samba is being provided for it, but the quality of it on 32bits is, well, questionable. So we either should draw this line or fix bugs. I dunno if it's good idea to push the change upstream though: history tells me this is nearly hopeless to perform changes in samba code even if the changes are small and obvious.. but this is entirely different story. Thanks for your good will and effort, nonetheless. You're welcome. Thanks! /mjt
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
On 17 November 2022 8:38 PM, Michael Tokarev wrote: > Do you have ability to compile samba? If yes (maybe with a debian package), > can you please try the attached patch? It removes the useless and actually > wrong check for the off_t overflowing size_t. > Attachments: > * fruit-disable-useless-size_t-overflow-check.patch Thanks for the patch Michael, I compiled and ran and although it got out of the disk size functions this time, it still slipped up with ../../source3/smbd/service.c:787(make_connection_snum) make_connection_snum: canonicalize_connect_path failed for service TimeMachineBackup, path /mnt/HDD/TimeMachine I do appreciate you cutting this patch for this issue, however I feel like I've been down this road before with these issues. Push a fix here, and if a new issue doesn't pop up over there straight away, give it a version or three and it will eventually. Hence my reference to whack-a-mole. As Andrew points out, this is somewhat outlying as test-cases go. There are few people wanting to rebuild a 32-bit instance of their backup mechanism for each point revision of a large and actively-developed service like samba. I see no reason, personally, to push for developer attention when the solution is as simple as 'switch to 64-bit OS', and then even OLD versions in all the repos work just fine. Although I do not have statistics to hand, I feel that we must be far enough along the migration curve on the journey from 32- to 64-bit systems, especially when it comes to a core-yet-commodity infrastructure service like samba, to warrant letting go of legacy. Thanks for your good will and effort, nonetheless.
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
17.11.2022 14:09, ArtMG wrote:
..
I have now reproduced the error on 4.13.13 on 32bit, and confirmed it is *still* an issue
with 4.16.7 on 32bit. However when I switch to 64bit OS version, the error does *not*
occur, not even in 4.13.13. The TimeMachine client successfully mounts the share and
completes multiple backups, and the server smbd logs are clean, with no overflow
warnings, and definitely no "Invalid or incomplete multibyte or wide character"
errors.
Do you have ability to compile samba? If yes (maybe with a debian package),
can you please try the attached patch? It removes the useless and actually
wrong check for the off_t overflowing size_t.
Or alternatively I can provide pre-built binaries.
Thanks,
/mjtdiff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 4058d4834e7..8e31e74f2a6 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -5273,17 +5273,6 @@ static bool fruit_tmsize_do_dirent(vfs_handle_struct *handle,
return true;
}
- /*
- * Arithmetic on 32-bit systems may cause overflow, depending on
- * size_t precision. First we check its unlikely, then we
- * force the precision into target off_t, then we check that
- * the total did not overflow either.
- */
- if (bandsize > SIZE_MAX/nbands) {
- DBG_ERR("tmsize potential overflow: bandsize [%zu] nbands [%zu]\n",
- bandsize, nbands);
- return false;
- }
tm_size = (off_t)bandsize * (off_t)nbands;
if (state->total_size + tm_size < state->total_size) {
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
17.11.2022 14:09, ArtMG wrote: Please check if 4.16 fixes this. If not, I guess the best course is to switch to a 64bit system, since apparently samba is having inherent issues on 32bit systems. I have now reproduced the error on 4.13.13 on 32bit, and confirmed it is *still* an issue with 4.16.7 on 32bit. However when I switch to 64bit OS version, the error does *not* occur, not even in 4.13.13. The TimeMachine client successfully mounts the share and completes multiple backups, and the server smbd logs are clean, with no overflow warnings, and definitely no "Invalid or incomplete multibyte or wide character" errors. I looked at the source of vfs_fruit module. It looks to me this whole thing is just trivial really. First, they use system off_t type for internal thing. It is not used for actual system functions which expect an offset in a file, it is used internally and/or in the protocol, in particular to report disk free space, - there, an off_t should not be used. For this reason, something like u_int64_t (or u_int_least64_t) type should be used instead of off_t with an unknown size. And second, when samba is built, LFS (large file support) should be enabled, so off_t should already be 64bits. To me it looks like somewhat questionable programming habits. For an uint type, something like ~((uint64_t)0) (I havn't done much programming in some 10 years - something *like* that) should give you the maximum value this type can ever hold, - that's SIZE_MAX for you. I'm not sure what this very test gives you in the first place. the max size *should* be able to hold a 64bit integer. FWIW. /mjt
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
> Please check if 4.16 fixes this. If not, I guess the best course is to
> switch to a 64bit system, since apparently samba is having inherent
> issues on 32bit systems.
I have now reproduced the error on 4.13.13 on 32bit, and confirmed it is
*still* an issue with 4.16.7 on 32bit. However when I switch to 64bit OS
version, the error does *not* occur, not even in 4.13.13. The TimeMachine
client successfully mounts the share and completes multiple backups, and the
server smbd logs are clean, with no overflow warnings, and definitely no
"Invalid or incomplete multibyte or wide character" errors.
> Can we at least retitle this as "... on 32bi platforms",
> since what you describe suggest that it's not specific
> to 32bits.
I concur, mjt, that the new bug title more accurately describes the issue, now
> I for one don't know what vfs_fruit *is*, to begin with. Just read briefly the
> manpage, -- well, it has quite some things, it seems, most of which is related
> to MacOS. I don't have a MacOS machine.
So, yes, vfs_fruit (named to indicate something apple-like without TM issues)
is a module that extends samba to provide macOS-related features - essentially
to serve SMB in the way that a Mac would. That means to test any issues one
would indeed need some kind of client Mac hardware or simulator available.
> If Samba is to have long-term 32 bit support someone needs to provide
> an upstream patch to run a docker build in 32 bit, ie a 32 bit
> userspace on 64 bit docker hosts.
Thank you for the wider upstream, persepctive, Andrew. In this particular case,
tests also required a terrabyte of storage on hand so that the volume
calculations would overflow. In my domestic one-off tests it was easy to lay my
hands on an old external HDD and a Raspberry Pi. However in a larger-team,
keeping a virtual setup for regression testing, and including the mac client
for testing vfs_fruit features – I can see how that is going to make for much
tougher logistics.
I guess the question now, it whether this gets marked as 'won't fix', because
it only occurs in these specific circumstances ("fruit:time machine max size"
is set) and for 32 bit systems, or whether it needs logging upstream? The
upstream development cost is likely to be low for this specific bug, but its
the ongoing commitment to testing that could weigh more heavily.
Personally I'm happy to migrate to 64bit and leave all the overflows behind me,
but I'll leave it to you higher-level project experts to make the important
decision. Thanks for all your work.
ArtMG
Bug#974868: [Pkg-samba-maint] Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
On Wed, 2022-11-16 at 18:29 +, ArtMG wrote: > Hi, > > > On Mon, 31 Oct 2022 14:19:24 +0300 Michael Tokarev > wrote: > > > Lacking any further information, I'll close this bugreport as > fixed in 4.16. > > I have just built and tested against 4.16.7 and I'm afraid to say > that the reported issue DOES still occur. > > > On Tue, 15 Nov 2022 11:03:02 +0300 Michael Tokarev > wrote: > > If you think this is incorrect and the issue is still here with > current version > > > of samba, please feel free to reopen this bug report with any extra > information > > which might be helpful to identify the issue. > > What kind of extra information might be helpful? I can pull > diagnostics off my test rig if it helps. > > > Part of me wants to get this issue resolved so that 32-bit systems > can meet these use-cases. However, I know from working on the patch > for that other bug ( > https://bugzilla.samba.org/show_bug.cgi?id=13622#c10) that you > increase the precision in one part of the code, and sooner or later > an issue arises in another. > > In the end, there's a limit how long it's pragmatic to play bug > whack-a-mole like this. Since my previous issue I have upgraded to > hardware that supports 64-bit, so I'm now off to validate whether > your suggestion of upgrading OS architecture can make this issue > magically disappear. If Samba is to have long-term 32 bit support someone needs to provide an upstream patch to run a docker build in 32 bit, ie a 32 bit userspace on 64 bit docker hosts. This should be entirely possible, but web searches show up very little, and even 32 bit VMs seem hard to find (Douglas specially built one years ago for a xsltproc doc build issue, but it was a hastle on most cloud providers). Bonus points if that 32 bit host is actually FreeBSD (to catch that as well) somehow running under docker via qemu yet still behaving as a subprocess, but that's off topic here. Andrew, -- 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
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
Control: retitle -1 samba: can't serve size-limited Time Machine shares on 32bit architectures Control: tag -1 + confirmed upstream Control: severity -1 minor 16.11.2022 21:29, ArtMG wrote: Hi, > On Mon, 31 Oct 2022 14:19:24 +0300 Michael Tokarev mailto:m...@tls.msk.ru>> wrote: > > Lacking any further information, I'll close this bugreport as fixed in 4.16. I have just built and tested against 4.16.7 and I'm afraid to say that the reported issue *DOES* still occur. Ok. It's good I didn't actually close this bug report as I wanted to - I thought about sending the email to nnn-done@bugs.d.o, but forgot about this -done part. (I was dealing with old bugs, there are *lots* of them reported against samba package, they're just sitting there for years without any action or attention, so something has to be done with them anyway). On Tue, 15 Nov 2022 11:03:02 +0300 Michael Tokarev mailto:m...@tls.msk.ru>> wrote: > If you think this is incorrect and the issue is still here with current version > of samba, please feel free to reopen this bug report with any extra information > which might be helpful to identify the issue. What kind of extra information might be helpful? I can pull diagnostics off my test rig if it helps. I for one don't know what vfs_fruit *is*, to begin with. Just read briefly the manpage, -- well, it has quite some things, it seems, most of which is related to MacOS. I don't have a MacOS machine. So basically, I know nothing about how to verify this. And the thing is that I can't really do anything there. It's best to be handled upstream anyway, I don't know samba internals. I can handle packaging issues, but for the real code issues, especially the ones which I can't even verify myself - I can only close the bug reports so the reports wont stack and hide somethin which I really can fix. Can we at least retitle this as "... on 32bi platforms", since what you describe suggest that it's not specific to 32bits. Part of me wants to get this issue resolved so that 32-bit systems can meet these use-cases. However, I know from working on the patch for that other bug (https://bugzilla.samba.org/show_bug.cgi?id=13622#c10) that you increase the precision in one part of the code, and sooner or later an issue arises in another. Umm. So we've hit some internal limitations with off_t size here. That's understandable. Just yesterday we talked with Qemu folks - they just *detest* 32bit platforms, since it's extremly difficult to map even the 32bit address space into its own 32bit address space, they have to perform all kinds of tricks which doesn't work anyway in the end, - it is just can not be done. In the end, there's a limit how long it's pragmatic to play bug whack-a-mole like this. Since my previous issue I have upgraded to hardware that supports 64-bit, so I'm now off to validate whether your suggestion of upgrading OS architecture can make this issue magically disappear. I see. Well, this is at least practical, I think. Thank you very much for your input! /mjt
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
Hi, > On Mon, 31 Oct 2022 14:19:24 +0300 Michael Tokarev wrote: > > Lacking any further information, I'll close this bugreport as fixed in 4.16. I have just built and tested against 4.16.7 and I'm afraid to say that the reported issue *DOES* still occur. On Tue, 15 Nov 2022 11:03:02 +0300 Michael Tokarev wrote: > If you think this is incorrect and the issue is still here with current > version > of samba, please feel free to reopen this bug report with any extra > information > which might be helpful to identify the issue. What kind of extra information might be helpful? I can pull diagnostics off my test rig if it helps. Part of me wants to get this issue resolved so that 32-bit systems can meet these use-cases. However, I know from working on the patch for that other bug (https://bugzilla.samba.org/show_bug.cgi?id=13622#c10) that you increase the precision in one part of the code, and sooner or later an issue arises in another. In the end, there's a limit how long it's pragmatic to play bug whack-a-mole like this. Since my previous issue I have upgraded to hardware that supports 64-bit, so I'm now off to validate whether your suggestion of upgrading OS architecture can make this issue magically disappear. Thanks ArtMG
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
Version: 2:4.16.0+dfsg-1 On Mon, 31 Oct 2022 14:19:24 +0300 Michael Tokarev wrote: .. Lacking any further information, I'll close this bugreport as fixed in 4.16. Let's close it now, it looks like there's no point to keep this bug report open. If you think this is incorrect and the issue is still here with current version of samba, please feel free to reopen this bug report with any extra information which might be helpful to identify the issue. Thanks, /mjt
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
Control: tag -1 + moreinfo On Fri, 8 Apr 2022 09:11:27 +0300 Michael Tokarev wrote: Control: tag -1 - patch On Tue, 15 Feb 2022 17:54:04 +0100 Daniel Gassen wrote: > Package: samba-vfs-modules > Version: 2:4.13.13+dfsg-1~deb11u3 > Followup-For: Bug #974868 > X-Debbugs-Cc: dan...@gassen.io > > Dear Maintainer, > > unfortunately this bug doesn't seem to be fixed. In that case this is some other issue, not the one mentioned in this bug (https://bugzilla.samba.org/show_bug.cgi?id=13622). Removing the tag (patch). Other than that, I don't have any good news to share, unfortunately. Please check if 4.16 fixes this. If not, I guess the best course is to switch to a 64bit system, since apparently samba is having inherent issues on 32bit systems. See #1006935 for another issue which appears to be 32bit-related too - http://bugs.debian.org/1006935 . The 32bit issues mentioned has been due to another, unrelated problem. But the question still persist: does this still occur with current version of samba, say, 4.16 from bullseye-backports? Lacking any further information, I'll close this bugreport as fixed in 4.16. Thanks, /mjt
Bug#974868: samba-vfs-modules: Still causing issues - at least on armv5tel/armel
Control: tag -1 - patch On Tue, 15 Feb 2022 17:54:04 +0100 Daniel Gassen wrote: Package: samba-vfs-modules Version: 2:4.13.13+dfsg-1~deb11u3 Followup-For: Bug #974868 X-Debbugs-Cc: dan...@gassen.io Dear Maintainer, unfortunately this bug doesn't seem to be fixed. In that case this is some other issue, not the one mentioned in this bug (https://bugzilla.samba.org/show_bug.cgi?id=13622). Removing the tag (patch). Other than that, I don't have any good news to share, unfortunately. Please check if 4.16 fixes this. If not, I guess the best course is to switch to a 64bit system, since apparently samba is having inherent issues on 32bit systems. See #1006935 for another issue which appears to be 32bit-related too - http://bugs.debian.org/1006935 . Thanks, /mjt
Bug#974868:
Hi, Le ven. 11 déc. 2020 à 07:12, Reid Priedhorsky a écrit : > > I was able to build and install a set of .debs including the above patch, > thanks to Raphael Hertzog’s fine instructions [2]. However, unfortunately I > still have the following error in the logs: > > [2020/12/10 22:25:23.078271, 0] ../source3/smbd/dfree.c:125(sys_disk_free) > sys_disk_free: VFS disk_free failed. Error was : Invalid or incomplete > multibyte or wide character > > So possibly the patch is insufficient? > > [1]: > https://raphaelhertzog.com/2011/07/04/how-to-prepare-patches-for-debian-packages/ Can you test the version in testing? The bug should be fixed there. Regards Mathieu Parent -- Mathieu
Bug#974868:
I was able to build and install a set of .debs including the above patch, thanks to Raphael Hertzog’s fine instructions [2]. However, unfortunately I still have the following error in the logs: [2020/12/10 22:25:23.078271, 0] ../source3/smbd/dfree.c:125(sys_disk_free) sys_disk_free: VFS disk_free failed. Error was : Invalid or incomplete multibyte or wide character So possibly the patch is insufficient? [1]: https://raphaelhertzog.com/2011/07/04/how-to-prepare-patches-for-debian-packages/
Bug#974868: samba: can't serve size-limited Time Machine shares on i386
Package: samba Version: 2:4.9.5+dfsg-5+deb10u1 Severity: important Tags: patch Hi, With the following line on a share in smb.conf: fruit:time machine max size = 640G authentication fails on my Mac when attempting to mount the share. This appears in the log: fruit_tmsize_do_dirent: tmsize overflow I'm pretty sure the problem is an instance of this upstream bug: https://bugzilla.samba.org/show_bug.cgi?id=13622 which was fixed with this patch: https://bugzilla.samba.org/attachment.cgi?id=15850 The share mounts and Time Machine is happy with it if I remove the configuration line noted. I believe the bug is fixed in the testing and unstable versions of the package, but I haven't verified this. The testing package has unmet dependencies on stable and there seems to be no backport. I am running multi-arch but the amd64 package had dependency problems I couldn't figure out. So the request is to apply the (quite short) patch to the stable version. I tagged it important because serving Time Machine shares is a common use of Samba and without this option, I can't put the share on a common filesystem without the backups growing unbounded. In my case, I have one Time Machine backup that's excessively large already and I want to keep it from getting worse. Please let me know what additional information I can provide to help. Thanks for your hard work on Debian! This particular box was originally installed around 1998 and upgraded smoothly since then, which speaks very well for the project IMO. Reid -- Package-specific info: * /etc/samba/smb.conf present, and attached * /var/lib/samba/dhcp.conf present, and attached -- System Information: Debian Release: 10.6 APT prefers stable-updates APT policy: (500, 'stable-updates'), (500, 'stable') Architecture: i386 (x86_64) Foreign Architectures: amd64 Kernel: Linux 4.19.0-12-amd64 (SMP w/12 CPU cores) Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages samba depends on: ii adduser 3.118 ii dpkg 1.19.7 ii libbsd0 0.9.1-2 ii libc6 2.28-10 ii libldb1 2:1.5.1+really1.4.6-3 ii libpam-modules1.3.1-5 ii libpam-runtime1.3.1-5 ii libpopt0 1.16-12 ii libpython2.7 2.7.16-2+deb10u1 ii libtalloc22.1.14-2 ii libtdb1 1.3.16-2+b1 ii libtevent00.9.37-1 ii lsb-base 10.2019051400 ii procps2:3.3.15-2 ii python2.7.16-1 ii python-dnspython 1.16.0-1 ii python-samba 2:4.9.5+dfsg-5+deb10u1 ii python2.7 2.7.16-2+deb10u1 ii samba-common 2:4.9.5+dfsg-5+deb10u1 ii samba-common-bin 2:4.9.5+dfsg-5+deb10u1 ii samba-libs2:4.9.5+dfsg-5+deb10u1 ii tdb-tools 1.3.16-2+b1 Versions of packages samba recommends: ii attr1:2.4.48-4 ii logrotate 3.14.0-4 ii samba-dsdb-modules 2:4.9.5+dfsg-5+deb10u1 ii samba-vfs-modules 2:4.9.5+dfsg-5+deb10u1 Versions of packages samba suggests: pn bind9 pn bind9utils pn ctdb pn ldb-tools pn ntp | chrony pn smbldap-tools pn ufw pn winbind -- no debconf information # # Sample configuration file for the Samba suite for Debian GNU/Linux. # # # This is the main Samba configuration file. You should read the # smb.conf(5) manual page in order to understand the options listed # here. Samba has a huge number of configurable options most of which # are not shown in this example # # Some options that are often worth tuning have been included as # commented-out examples in this file. # - When such options are commented with ";", the proposed setting #differs from the default Samba behaviour # - When commented with "#", the proposed setting is the default #behaviour of Samba but the option is considered important #enough to be mentioned here # # NOTE: Whenever you modify this file you should run the command # "testparm" to check that you have not made any basic syntactic # errors. #=== Global Settings === [global] # Time Machine stuff min protocol = SMB2 fruit:time machine = yes fruit:aapl = yes fruit:delete_empty_adfiles = yes ## Browsing/Identification ### # Change this to the workgroup/NT-domain name your Samba server will part of workgroup = JESSNET Networking # The specific set of interfaces / networks to bind to # This can be either the interface name or an IP address/netmask; # interface names are normally preferred ; interfaces = 127.0.0.0/8 eth0 # Only bind to the named interfaces and/or networks; you must use the # 'interfaces' option above to use this. # It is recommended that you enable this feature if your Samba machine is # not protected by a firewall or is a firewall itself. However, this # option cannot handle
