#221 64 bit time_t breaks radmind server

[Hauke Fath]

NetBSD 6 will ship with a 64 bit time_t, solving UNIXes year 2038 problem. Unfortunately, Radmind implicitely assumes sizeof(time_t) == 4.

The NetBSD PR 46555 at "http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=46555" has the details.

From here, I see two ways out: Teach the fprintf()-like snet functions about %jd, and cast the st_mtime field to (intmax_t), or come up with something like PRIofft, say: 'PRItimet', for time_t.

[Hauke Fath]

See also the "va_arg and netbsd-6" thread on the NetBSD pkgsrc-users list
"http://comments.gmane.org/gmane.os.netbsd.devel.pkgsrc.user/16521"

[Andrew Mortensen]

Thanks for the detailed report.I think the PRItimet option looks better.

[Hauke Fath]

"largefile.h" lookalike for 64 bit time_t

[Hauke Fath]

Looks better to me, too, especially since it is the path taken by off_t support. Attached a largefile.h lookalike, missing is the autotools support for detecting time_t size.

[Andrew Mortensen]

Thanks. I've added the check for time_t size to configure.ac, and am going through the project to replace 32-bit time_t conversions with a strtotimet macro.

[Andrew Mortensen]

First pass at patch in master branch. Please pull from git, test and report here.

Commit 9c355a17df26c1e05cc2132fef3c9cd7bfe2e6a1.

[Hauke Fath]

Thanks for working on the problem.

Unfortunately, I will be AFK from tomorrow until early October. I'll get back to you then.

[Hauke Fath]

Sorry, it took a while to come back, then wrap my mind around checking out and building from git. I have a package now that checks out from git, and may commit it to pkgsrc-wip.

Unfortunately, the server still has issues:

[hf@Hochstuhl] ~ # cd /var/obj/pkgsrc/hauke/radmind-git/work/radmind/
[hf@Hochstuhl] /<5>work/radmind # gdb radmind
GNU gdb (GDB) 7.3.1
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486--netbsdelf".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /var/obj/pkgsrc/hauke/radmind-git/work/radmind/radmind...done.
(gdb) set args -D /u/radmind -w1 -z /u/radmind/cert/radmind.key -d -f
(gdb) set follow-fork-mode child
(gdb) run
Starting program: /var/obj/pkgsrc/hauke/radmind-git/work/radmind/radmind -D /u/radmind -w1 -z /u/radmind/cert/radmind.key -d -f
<<< STARTTLS
<<< STAT COMMAND

Program received signal SIGSEGV, Segmentation fault.
[Switching to process 13573]
0x08051ddd in snet_writeftv (sn=0xbb73c180, tv=0x0, format=0x80540f3 "s\r\n") at snet.c:275
275 len = strlen( p );
(gdb) bt
#0 0x08051ddd in snet_writeftv (sn=0xbb73c180, tv=0x0, format=0x80540f3 "s\r\n") at snet.c:275
#1 0x0804d6a3 in f_stat (sn=0xbb73c180, ac=2, av=0xbb71d1f0) at ./command.c:691
#2 0x0804e57f in cmdloop (fd=5, sin=0xbfbfe7fc) at ./command.c:1627
#3 0x0804b3f8 in main (ac=8, av=0xbfbfe890) at ./daemon.c:623
(gdb) print p
$1 = 0x0
(gdb)

where snet.c:275 is

270 modifier:
271
272 switch ( *++format ) {
273 case 's' :
274 p = va_arg( vl, char * );
275 len = strlen( p );
276 SNET_WBUFGROW( len );
277 memcpy( cur, p, len );
278 cur += len;
279 break;

[Hauke Fath]

It turned out that the time_t related entry was missing from config.h.in ...

See http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/radmind/patches/patch-config.h.in?rev=1.1, and you might also want the check for SIZEOF_TIME_T in http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/radmind/patches/patch-largefile.h?rev=1.1.

With the above patches, HEAD builds and runs fine on NetBSD 6.

BTW, the build system does not ensure libsnet has been built before linking to it, and parallel makes fail.

Given the size of the patches necessary to the 1.14.3 package (see http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/radmind/patches/), I guess a new release would be helpful?

Cheerio,
hauke

[Hauke Fath]

The command.c part of commit [9c355a] introduces a bug.

It replaces the printf format strings with RADMIND_STAT_FMT, but the last snet_writef()'s format string (setting custom permissions and ownership) is different from the others. The code ends up reading string chars as int values, resulting in garbled permission and ownership. This can be a security issue.

For a corrected patch, please see http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/radmind/patches/patch-command.c?rev=1.2&content-type=text/x-cvsweb-markup.

Cheerio,
Hauke