Skip to content

Conversation

@alejandro-colomar
Copy link
Collaborator

These are the patches that I reviewed from that PR.

Cc: @stoeckmann

SCALE is always DAY (and has to be always DAY), so replace it with DAY
in source code and remove unneeded calculations.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <shadow-maint#876>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

v1b changes:

  • Rebase to master
  • Add link
$ git range-diff ddbd3a36..gh/ts shadow/master..ts
1:  3699966d ! 1:  1f022177 lib/, src/: Remove SCALE definition
    @@ Commit message
         in source code and remove unneeded calculations.
     
         Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
    +    Link: <https://github.com/shadow-maint/shadow/pull/876>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/age.c ##
2:  f1743d43 ! 2:  ac6cb8ad src/passwd.c: Switch to day precision
    @@ Commit message
         signed integer overflow.
     
         Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
    +    Link: <https://github.com/shadow-maint/shadow/pull/876>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/passwd.c ##
3:  c0a463ea ! 3:  05fd4899 src/passwd.c: Add overflow check
    @@ Commit message
         src/passwd.c: Add overflow check
     
         Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
    +    Link: <https://github.com/shadow-maint/shadow/pull/876>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/passwd.c ##
4:  cdeb8852 ! 4:  0ac323dc src/: Fix long/time_t handling
    @@ Commit message
         since their long data type is still 32 bit.
     
         Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
    +    Link: <https://github.com/shadow-maint/shadow/pull/876>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/pwck.c ##

The size of time_t varies across systems, but since data type long is
more than enough to calculate with days (precision of shadow file),
use it instead.

Just in case a shadow file contains huge values, check for a possible
signed integer overflow.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <shadow-maint#876>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 30, 2023

v2 changes:

  • Remove space
  • Use __builtin_add_overflow(), which is safer than __builtin_saddl_overflow() (the latter may silently truncate some value while converting to long, and provides no benefit at all).
$ git range-diff shadow/master gh/ts ts
1:  1f022177 = 1:  1f022177 lib/, src/: Remove SCALE definition
2:  ac6cb8ad ! 2:  6440e0ab src/passwd.c: Switch to day precision
    @@ src/passwd.c: static void check_password (const struct passwd *pw, const struct
     -          time_t ok;
     -          ok = (time_t) sp->sp_lstchg * DAY;
     +          long now, ok;
    -+          now = time (NULL) / DAY;
    ++          now = time(NULL) / DAY;
     +          ok = sp->sp_lstchg;
                if (sp->sp_min > 0) {
     -                  ok += (time_t) sp->sp_min * DAY;
3:  05fd4899 < -:  -------- src/passwd.c: Add overflow check
-:  -------- > 3:  ec794928 src/passwd.c: Add overflow check
4:  0ac323dc = 4:  711b8a27 src/: Fix long/time_t handling

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <shadow-maint#876>
Co-developed-by: Alejandro Colomar <alx@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Special care has to be taken for 32 bit systems with a 64 bit time_t,
since their long data type is still 32 bit.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <shadow-maint#876>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

v2b changes:

  • Update commit message
$ git range-diff shadow/master gh/ts ts
1:  1f022177 = 1:  1f022177 lib/, src/: Remove SCALE definition
2:  6440e0ab = 2:  6440e0ab src/passwd.c: Switch to day precision
3:  ec794928 ! 3:  93db9c97 src/passwd.c: Add overflow check
    @@ Commit message
     
         Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
         Link: <https://github.com/shadow-maint/shadow/pull/876>
    +    Co-developed-by: Alejandro Colomar <alx@kernel.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/passwd.c ##
4:  711b8a27 = 4:  2e63ceeb src/: Fix long/time_t handling

@hallyn hallyn merged commit 1a38319 into shadow-maint:master Jan 5, 2024
alejandro-colomar pushed a commit that referenced this pull request Feb 14, 2024
SCALE is always DAY (and has to be always DAY), so replace it with DAY
in source code and remove unneeded calculations.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: <#876>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Cherry-picked-from: ecc3508 ("lib/, src/: Remove SCALE definition")
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Link: <#888>
Link: <#876>
[alx: This is a pre-requisite for 674409e ("lib/: Saturate addition to avoid overflow")]
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants