Re: can not download IMAP messages with isync/mbsync
> And, the wraparound seems to happen at 0x7fff instead of > 0x. Don't know ARM well enough to explain why. It's probably using a signed, instead of unsigned, conditional branch instruction. (I think for ARM it's the branch rather than the compare that differs for signed vs unsigned.) If the ARM ABI can place data both above and below the 0x8000 divide, that's another bug waiting to happen in the ARM assembly strnlen; it will misbehave for a string that crosses that point, even when given a non-ludicrous second argument. But I suspect it really should just get rid of the "end = str + maxlen;" and "ptr < end" paradigm altogether, whether or not it's written in assembly. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: can not download IMAP messages with isync/mbsync
On Mon, 14 Nov 2022, RVP wrote:
On Mon, 14 Nov 2022, Mouse wrote:
My guess is that the buffer you're testing with is near the top of the
address space, within ~1GB of address 0x, and what you're
seeing is due to wraparound.
Thanks for that analysis--address-wrapping was my first guess too, but,
I didn't have the time to confirm it: the 1GB was with a standalone
program; in mbsync itself, the range was much smaller--less than 1MB even.
Well, that 1GB vs 1MB difference is easily explained. In my test code,
below, I originally had `const char* const s = "hello"'. So, `s' pointed
to a data segment address instead of the stack.
And, the wraparound seems to happen at 0x7fff instead of 0x.
Don't know ARM well enough to explain why.
---START---
#include
#include
#include
#include
static void
f2(const char* fmt, va_list ap)
{
size_t max, l = 0, n;
const char* s;
if (*fmt != 's')
return;
s = va_arg(ap, const char *);
n = strlen(s);
for (max = ULONG_MAX; max != 0; max /= 2)
if ((l = strnlen(s, max)) == n)
break;
printf("s @ %p, got len=%zu when max=%zu\n", s, l, max);
}
static void
f1(const char* fmt, ...)
{
va_list ap;
va_start(ap, fmt);
f2(fmt, ap);
va_end(ap);
}
int
main(void)
{
const char s[] = "hello";
f1("s", s);
return 0;
}
---END---
-RVP
Re: can not download IMAP messages with isync/mbsync
>> My guess is that the buffer you're testing with is near the top of >> the address space, within ~1GB of address 0x, and what >> you're seeing is due to wraparound. > Thanks for that analysis--address-wrapping was my first guess too, > but, I didn't have the time to confirm it: the 1GB was with a > standalone program; in mbsync itself, the range was much > smaller--less than 1MB even. I haven't confirmed it myself. I don't have an ARM machine running anything more recent than 4.0.1 (and that much only quite recently - I found my shark in storage and am only just getting it back in full operation). 4.0.1 appears to not even _have_ strnlen. But my reading of the assembly code I found in 9.1's /usr/src matches the behaviour you describe far too well for me to think it's entirely coincidence; I'm fairly fairly confident of my analysis. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: can not download IMAP messages with isync/mbsync
On Mon, 14 Nov 2022, Mouse wrote: My guess is that the buffer you're testing with is near the top of the address space, within ~1GB of address 0x, and what you're seeing is due to wraparound. Thanks for that analysis--address-wrapping was my first guess too, but, I didn't have the time to confirm it: the 1GB was with a standalone program; in mbsync itself, the range was much smaller--less than 1MB even. Also, - uint maxlen = UINT_MAX; + uint maxlen = sizeof(buf); if maxlen is passed unchanged to strnlen, I can't see how the original code isn't a bug; there's no point in using strnlen if you're pass a maxlen greater than the space remaining in the buffer your pointer points into. It _is_ passed as-is to strnlen(). I chose sizeof(buf) instead of UINT_MAX because `buf' seemed sized for the RFC-5322 recommended line-length limit of 998 octets. The 9.1 manpage for strnlen says The strnlen() function returns either the same result as strlen() or maxlen, whichever is smaller. which makes this a violation of its spec. Right, then Marko can file a PR so that this can be fixed a) either in the ARM assembly, or b) by NetBSD-ARM just using the C version like the other ports. Thx, -RVP
Re: can not download IMAP messages with isync/mbsync
> Or is UINT_MAX not guaranteed to fit in size_t
I _think_ there is no guarantee that UINT_MAX fits in a size_t. But,
upthread, I see...
> Turn out, on ARM, strnlen(3) is written in assembly and this always
> returns `maxlen' for any value of `maxlen' > ~1GB.
Not quite.
I have a guest login on a 9.1 machine, and found the ARM strnlen there.
I am not an ARM expert, but I know it enough to, I think, find and
explain the bug.
My guess is that the buffer you're testing with is near the top of the
address space, within ~1GB of address 0x, and what you're
seeing is due to wraparound.
Here's the relevant code (from 9.1):
addsr5, r0, r1 /* get ptr to end of string */
mov r4, r1 /* save maxlen */
...
.Lmain_loop:
#ifdef STRNLEN
cmp r0, r5 /* gone too far? */
bge .Lmaxed_out /* yes, return maxlen */
#endif
(The code at .Lmaxed_out just returns maxlen, as the comment implies.)
Back-translating loosely into C, what we have here is
strnlen(const char *buf, int maxlen)
{
const char *end;
end = buf + maxlen;
...
while (1) {
if (buf > end)
return(maxlen);
...
}
}
This back-translation is, of course, broken from a C perspective, but
it's supposed to be illustrative, not precise. The bug: if buf+maxlen
overflows (at the machine-code level, on ARM32, buf, maxlen, and end
are each just 32-bit integers), then buf>end can be true right from the
start, terminating the loop (and returning maxlen) before it should.
The 9.1 manpage for strnlen says
The strnlen() function returns either the same result as strlen() or
maxlen, whichever is smaller.
which makes this a violation of its spec. The only way it could be
non-broken is if size_t's range and the address space layout
collaborate to ensure that string + maxlen can't wrap around. Since I
think both are 32-bit on (32-bit) ARM, this isn't so.
Also,
- uint maxlen = UINT_MAX;
+ uint maxlen = sizeof(buf);
if maxlen is passed unchanged to strnlen, I can't see how the original
code isn't a bug; there's no point in using strnlen if you're pass a
maxlen greater than the space remaining in the buffer your pointer
points into. I'd have to look at more of the surrounding code to be
sure, though. (It also depends on nonportable assumptions about the
relative sizes of uint and size_t, but that bug is concealed on 32-bit
NetBSD.)
/~\ The ASCII Mouse
\ / Ribbon Campaign
X Against HTMLmo...@rodents-montreal.org
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: can not download IMAP messages with isync/mbsync
On Mon, Nov 14, 2022 at 09:48:50AM -0500, Greg Troxel wrote: > That's why it is harder to meet the spec than it first seems. C > doesn't offer UB for such arguments, so it should be fixed. (I'm not > asking anyone to do the work - just to agree it's broken.) I am not sure it is broken, but how would you fix it? Effectively you could only detect the out of bounds maxlen and return the value of strlen() instead - but that sounds like a super stupid idea. Martin
Re: can not download IMAP messages with isync/mbsync
Martin Husemann writes: > On Mon, Nov 14, 2022 at 08:17:32AM -0500, Greg Troxel wrote: >> I am not arguing against fixing the code to be sane. I am just raising >> the other question about maybe strnlen needs fixing. > > "s + maxlen" (for most s) wraps around on 32bit architectures so the > typical implementation will notice the current char * being >= that > limit and claim we ran too far. That's why it is harder to meet the spec than it first seems. C doesn't offer UB for such arguments, so it should be fixed. (I'm not asking anyone to do the work - just to agree it's broken.) signature.asc Description: PGP signature
Re: can not download IMAP messages with isync/mbsync
On Mon, Nov 14, 2022 at 08:17:32AM -0500, Greg Troxel wrote: > I am not arguing against fixing the code to be sane. I am just raising > the other question about maybe strnlen needs fixing. "s + maxlen" (for most s) wraps around on 32bit architectures so the typical implementation will notice the current char * being >= that limit and claim we ran too far. Martin
Re: can not download IMAP messages with isync/mbsync
Martin Husemann writes: > On Mon, Nov 14, 2022 at 01:22:54PM +0100, Marko Bauhardt wrote: >> I configured to not use the system strnlen as you suggested. this works fine. >> i'm able to fetch my mails. >> THX!! > > The patch is the better solution, the length passed to strnlen is > obviously bogus. Yes, it's crazy. But isn't strnlen failing to meet its specification? Or is UINT_MAX not guaranteed to fit in size_t and we're getting truncation? I am not arguing against fixing the code to be sane. I am just raising the other question about maybe strnlen needs fixing. signature.asc Description: PGP signature
Re: can not download IMAP messages with isync/mbsync
On Mon, Nov 14, 2022 at 01:22:54PM +0100, Marko Bauhardt wrote: > I configured to not use the system strnlen as you suggested. this works fine. > i'm able to fetch my mails. > THX!! The patch is the better solution, the length passed to strnlen is obviously bogus. Martin
Re: can not download IMAP messages with isync/mbsync
> RVP hat am 14.11.2022 08:46 CET geschrieben: hey rvp you are the greatest! awesome. > ...Turn out, on ARM, strnlen(3) is written in assembly > and this always returns `maxlen' for any value of `maxlen' > ~1GB. > The fix > is either: > > a) Configure isync-1.4.4 to _not_ use the system strnlen(). It will > then use its own implementation which seems to work fine. > > ``` > $ env ac_cv_func_strnlen=no ./configure ... > $ make > $ make install > ``` I configured to not use the system strnlen as you suggested. this works fine. i'm able to fetch my mails. THX!! I will cross post this to the isync-mailinglist to let the devs know what the actual issue is. thx marko
Re: can not download IMAP messages with isync/mbsync
On Mon, 7 Nov 2022, RVP wrote:
I've not been able to reproduce this at all even with 3 servers (2
providers and 1 local [dovecot +COMPRESS]) on 9.3_STABLE/amd64.
OK. Once I had QEMU + NetBSD-ARMv7 running, it turned out to be an easy
issue to diagnose. Turn out, on ARM, strnlen(3) is written in assembly
and this always returns `maxlen' for any value of `maxlen' > ~1GB. I
don't know any ARM assembly, so I haven't chased this any further.
The fix is either:
a) Configure isync-1.4.4 to _not_ use the system strnlen(). It will
then use its own implementation which seems to work fine.
```
$ env ac_cv_func_strnlen=no ./configure ...
$ make
$ make install
```
OR
b) Apply this patch:
```
diff -urN isync-1.4.4.orig/src/drv_imap.c isync-1.4.4/src/drv_imap.c
--- isync-1.4.4.orig/src/drv_imap.c 2021-12-03 10:56:16.0 +
+++ isync-1.4.4/src/drv_imap.c 2022-11-14 06:58:25.037251216 +
@@ -541,7 +541,7 @@
add_seg( s, l );
if (!c)
break;
- uint maxlen = UINT_MAX;
+ uint maxlen = sizeof(buf);
c = *++fmt;
if (c == '\\') {
c = *++fmt;
```
This restricts `maxlen' to the size of the buffer we're using. (The amt.
actually written will of course be smaller--the code takes care of that.)
-RVP
Re: can not download IMAP messages with isync/mbsync
On Sun, 6 Nov 2022, Marko Bauhardt wrote:
This give me the following error while `configure`
```
configure: error: compiler does not support required C11 features
```
This shouldn't happen. Do you any custom CC, CPPFLAGS, CFLAGS, LDFLAGS
set?
I‘m getting
´´´
Loading far side box...
F: [ 5] Enter load_box, [1,inf] (find >= 0, paired <= 4294967295, new > 0)
=
==20988==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62a8a979 at
pc 0x650d7c30 bp 0x7fe47f64 sp 0x7fe47b28
WRITE of size 10 at 0x62a8a979 thread T0
ASAN:DEADLYSIGNAL
AddressSanitizer: nested bug in the same thread, aborting.
´´´
ASAN:DEADLYSIGNAL indicates some kind of severe issue. The sanitizer
should've produced a call-trace instead of that.
I've not been able to reproduce this at all even with 3 servers (2
providers and 1 local [dovecot +COMPRESS]) on 9.3_STABLE/amd64.
Can you try with the patch below. Compile isync-1.4.4 like this:
```
unset CC
export CFLAGS="-O0 -g -fsanitize=address"
export CPPFLAGS=-I/usr/pkg/include
export LDFLAGS="-L/usr/pkg/lib -Wl,-rpath=/usr/pkg/lib"
tar -xf /tmp/isync-1.4.4.tar.gz
mkdir build-isync-1.4.4
cd build-isync-1.4.4
../isync-1.4.4/configure --prefix=/tmp/I
make
make install
```
This create a non-PIE debug executable which you can run/debug after
setting ASLR off: `sysctl -w security.pax.aslr.enabled=0'
See if the sanitizer error goes away and you get a proper call-trace.
As it is now, I doubt we can use that PC address (0x650d7c30) in GDB
and get a correct code (l)isting: it will most likely be in ASAN itself.
---START---
diff -urN isync-1.4.4.orig/src/drv_imap.c isync-1.4.4/src/drv_imap.c
--- isync-1.4.4.orig/src/drv_imap.c 2021-12-03 10:56:16.0 +
+++ isync-1.4.4/src/drv_imap.c 2022-11-07 21:57:49.646386142 +
@@ -2469,7 +2469,10 @@
cmd = new_imap_cmd( sizeof(*cmd) );
cmd->param.cont = do_sasl_auth;
- imap_exec( ctx, cmd, done_sasl_auth, enc ? "AUTHENTICATE %s %s" :
"AUTHENTICATE %s", gotmech, enc );
+ if (enc)
+ imap_exec( ctx, cmd, done_sasl_auth, "AUTHENTICATE %s
%s", gotmech, enc );
+ else
+ imap_exec( ctx, cmd, done_sasl_auth, "AUTHENTICATE %s",
gotmech );
free( enc );
return;
notsasl:
diff -urN isync-1.4.4.orig/src/util.c isync-1.4.4/src/util.c
--- isync-1.4.4.orig/src/util.c 2021-12-03 10:56:16.0 +
+++ isync-1.4.4/src/util.c 2022-11-07 22:08:45.526920483 +
@@ -353,6 +353,7 @@
if (blen <= 0 || (uint)(ret = vsnprintf( buf, (size_t)blen, fmt, va ))
>= (uint)blen)
oob();
va_end( va );
+ assert(ret >= 0);/* XXX: paranoia */
return ret;
}
@@ -368,6 +369,8 @@
{
void *ret;
+ if (sz == 0)
+ return NULL;/* XXX: avoid undefined behaviour */
if (!(ret = malloc( sz )))
oom();
return ret;
@@ -378,6 +381,8 @@
{
void *ret;
+ if (sz == 0)
+ return NULL;/* XXX: avoid undefined behaviour */
if (!(ret = calloc( sz, 1 )))
oom();
return ret;
@@ -388,6 +393,11 @@
{
char *ret;
+ if (sz == 0) { /* XXX: ape glibc behaviour */
+ if (mem)
+ free(mem);
+ return NULL;
+ }
if (!(ret = realloc( mem, sz )) && sz)
oom();
return ret;
---END---
-RVP
Re: can not download IMAP messages with isync/mbsync
> RVP hat am 05.11.2022 08:51 CET geschrieben: > Does this occur if you compile isync with `--without-zlib'? This give me the following error while `configure` ``` configure: error: compiler does not support required C11 features ``` > Try compiling with `-fsanitize=address -g -O0' I‘m getting ´´´ Loading far side box... F: [ 5] Enter load_box, [1,inf] (find >= 0, paired <= 4294967295, new > 0) = ==20988==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62a8a979 at pc 0x650d7c30 bp 0x7fe47f64 sp 0x7fe47b28 WRITE of size 10 at 0x62a8a979 thread T0 ASAN:DEADLYSIGNAL AddressSanitizer: nested bug in the same thread, aborting. ´´´ Marko
Re: can not download IMAP messages with isync/mbsync
On Fri, 4 Nov 2022, Marko Bauhardt wrote: I want to use isync (mbsync) on netbsd to download my mails from my IMAP account. isync with the same configuration works fine on OSX and linux, but fails on netbsd. There is already a discussion on * https://sourceforge.net/p/isync/bugs/64/ * https://sourceforge.net/p/isync/mailman/isync-devel/thread/YqcwQSVfaiuGLkhf%40ugly Does this occur if you compile isync with `--without-zlib'? I tried with 2 servers (neither supporting compression) and couldn't reproduce this. Do you have any suggestion how do you debug the memory on netbsd? Or is anyone from you using mbsync/isync on netbsd? Try compiling with `-fsanitize=address -g -O0' -RVP
can not download IMAP messages with isync/mbsync
Hey, I want to use isync (mbsync) on netbsd to download my mails from my IMAP account. isync with the same configuration works fine on OSX and linux, but fails on netbsd. There is already a discussion on * https://sourceforge.net/p/isync/bugs/64/ * https://sourceforge.net/p/isync/mailman/isync-devel/thread/YqcwQSVfaiuGLkhf%40ugly The outcome was to use `valgrind` to debug the memory usage to check if there is a memory issue. As far as I know `valgrind` is not available for netbsd. Do you have any suggestion how do you debug the memory on netbsd? Or is anyone from you using mbsync/isync on netbsd? Thx Marko
