shadow-maint/shadow

gettime() rejects >ULONG_MAX (Reproducible Builds problem)

alejandro-colomar opened this issue · 2 comments

alejandro-colomar commented

I've opened this issue to track the problem, since comments on commits aren't easy to handle [@hallyn ].

Please keep the discussion here. It started in cb610d5#r136407772, so please read that, but don't write there.

Cc: @lamby
Cc: @pkitszel

alejandro-colomar commented

@pkitszel

Hi, I see no replies on mailing list and the PR linked above is rather huge and still unmerged, so:

We're frozen for a release, due in a few days probably. After that, depending on the time @hallyn and @ikerexxe have for reviewing, I'll try to get this merged ASAP.

That PR looks huge, but it's really overlapping several other smaller PRs, so it shouldn't be too hard to review, I think. I've divided it in similar chunks that are related (each branch is a PR):

$ git log --oneline --graph alx/master..atoi 
* cf15e38c (HEAD -> atoi, gh/atoi) share/containers/: trap(1) to see the cmocka logs
* dc1d6caa share/containers/: Specify one argument per line
* 7cbf064a src/usermod.c: Use id_t for parsing uid_t and gid_t
* 4938172d src/usermod.c: Rename identifiers ralated to [ug]id's
* acea6f0d lib/atoi/: Use libbsd's strto[iu](3bsd) when available
* d7f4e5b7 lib/idmapping.c: get_map_ranges(): Don't exit() from a library function
* a5f2df7f lib/idmapping.c: get_map_ranges(): Merge two input checks into a simpler one
* 11a94545 lib/idmapping.c: get_map_ranges(): Simplify iterator variables
* 258a7c24 lib/idmapping.c: get_map_ranges(): Remove dead code
* fa9729f5 lib/idmapping.c: get_map_ranges(): Move range check to a2ul() call
* 236ccb2a lib/idmapping.c: get_map_ranges(): Move range check to a2ul() call
* 25c013dd lib/idmapping.c: get_map_ranges(): Move range check to a2ul() calls
* 0c1534d2 lib/idmapping.c: get_map_ranges(): Rename local variable
* 98e50181 lib/limits.c: check_logins(): Report LOGIN_ERROR_LOGIN if str2ul() ERANGE

* dba07826 (gh/getuh, getuh) src/chage.c: Simplify, by calling a2sl() instead of str2sl()
* 32e2bc72 src/faillog.c: Simplify, by calling str2sh() instead of str2sl()
* 4c7c7dac src/usermod.c: Simplify, by calling a2sl() instead of str2sl()
* ce03bd36 src/passwd.c: Simplify, by calling a2sl() instead of str2sl()
* 5720b8c3 src/useradd.c: Simplify, by calling a2sl() instead of str2sl()
* 9d46fb4d src/check_subid_range.c: Call get_uid() instead of str2sl()
* 3446ab6d lib/shadow.c: my_sgetspent(): Fix error handling
* fe467119 lib/shadow.c: my_sgetspent(): Remove dead code
* 3c2a41e5 lib/shadow.c: my_sgetspent(): Merge 'else {if}' into 'else if'
* 36e65990 lib/sgetspent.c: sgetspent(): Simplify, by calling a2sl() instead of str2sl()
* 82053f00 lib/limits.c: setup_limits(): Simplify, by calling str2i(mode_t, ) instead of str2ul()
* 44e2ff08 lib/limits.c: setup_limits(): Simplify, by calling str2si() instead of str2sl()
* cab51e0b lib/limits.c: setup_limits(): Simplify, by calling a2si() instead of str2sl()
* 1833e8a8 lib/limits.c: set_umask(): Simplify, by calling str2i(mode_t, ) instead of str2ul()
* 5ccad9d9 lib/limits.c: set_prio(): Simplify, by calling str2si() instead of str2sl()
* 8e13fe28 lib/getdef.c: getdef_long(): Simplify, by calling a2sl() instead of str2sl()
* 32c1697b lib/getdef.c: getdef_unum(): Fix wrong limit check
* 5644b13b lib/getdef.c: getdef_num(): Simplify, by calling a2si() instead of str2sl()

* 05152cc1 (gh/strtoll, strtoll) lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()
* 0beee62d lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
* bd2d3517 lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
* 2a3d21db src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
* 7fadf337 lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
* b00a9319 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
* 07a99a63 lib/: get_pid(): Move function to "atoi/getnum.h"
* 5a73210c lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
* af7aed0c lib/: Don't open-code get_gid()
* 7d68f37c lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
* 3478b6d9 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
* 9604d4d1 src/: Use str2[u]l() instead of atoi(3)
* 9741fe1d lib/typetraits.h: Add macros that give information about a type

* 4015fd5b (gh/rm_noneg, rm_noneg) lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function
* e2c4a6df src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
* 2deac267 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
* d67067d9 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()

* 53daccc7 (gh/getrange, getrange) lib/getrange.c: getrange(): Report an ERANGE error when min>max
* 65e8659d lib/getrange.c: getrange(): Add missing cast
* 70dd39b6 lib/getrange.c: getrange(): Use a2ul() instead of strtoul_noneg()
* 3aa494f2 lib/getrange.c: getrange(): Rename local variable
* 77a53f0b lib/getrange.c: getrange(): Reduce uses of non-const pointer
* 4b624452 lib/, src/: Rename some local variables
* 87a8c673 lib/getrange.c: getrange(): Use goto to deduplicate code
* f5a77ca3 lib/getrange.c: getrange(): Return early to reduce indentation
* 745e96f7 lib/getrange.c: getrange(): Return early to reduce indentation
* b5151da5 lib/getrange.c: getrange(): Remove temporary variable

* 15dc249b (gh/getlong, getlong) lib/atoi/: a2*(), str2*(): Add variants for other types
* 7b548177 lib/, src/: str2*(): Rename functions and reorder parameters
* ba573c4b lib/atoi/: Add a2[su]l() and reimplement get[u]long() in terms of them
* 1974802a lib/, src/, po/: get[u]long(): Move functions to lib/atoi/str2i.h

which results in the following PRs:

Also, liba2i is already available at

I'm still working on the build system a little bit so I didn't say anything yet, but if you're interested in packaging that, please let me know if I can help. Just mind that the source code is stable and tested, but the build system is still unstable, so don't rely too much on it (but you're welcome to try it, so I can improve it).

(BTW, @thesamesam , there's already a kernel.org git and a mailing list, so you can report your remaining concerns about it in the list)

alejandro-colomar commented

And if you're only interested in what changes gettime, it's just two commits:

$ git log --oneline --graph alx/master..atoi  -- lib/gettime.c
* d67067d9 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
* 4b624452 lib/, src/: Rename some local variables