Bug 30804 - F_GETLK, F_SETLK, and F_SETLKW value change for powerpc64 with -D_FILE_OFFSET_BITS=64
Summary: F_GETLK, F_SETLK, and F_SETLKW value change for powerpc64 with -D_FILE_OFFSET...
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: 2.39
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-28 21:07 UTC by Aurelien Jarno
Modified: 2023-09-16 09:11 UTC (History)
4 users (show)

See Also:
Host: powerpc64le-unknown-linux-gnu
Target: powerpc64le-unknown-linux-gnu
Build: powerpc64le-unknown-linux-gnu
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aurelien Jarno 2023-08-28 21:07:32 UTC

    
Comment 1 Aurelien Jarno 2023-08-28 21:15:40 UTC
On powerpc64le-unknown-linux-gnu the record locking constants values with -D_FILE_OFFSET_BITS=64 have changed between glibc 2.37 and glibc 2.38:

glibc 2.37:
- F_GETLK: 12
- F_SETLK: 13
- F_SETLKW: 14

glibc 2.38:
- F_GETLK: 5
- F_SETLK: 6
- F_SETLKW: 7

This is caused by the following commit:

commit 5f828ff824e3b7cd133ef905b8ae25ab8a8f3d66
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Tue May 30 16:40:38 2023 -0300

    io: Fix F_GETLK, F_SETLK, and F_SETLKW for powerpc64
    
    Different than other 64 bit architectures, powerpc64 defines the
    LFS POSIX lock constants  with values similar to 32 ABI, which
    are meant to be used with fcntl64 syscall.  Since powerpc64 kABI
    does not have fcntl, the constants are adjusted with the
    FCNTL_ADJUST_CMD macro.
    
    The 4d0fe291aed3a476a changed the logic of generic constants
    LFS value are equal to the default values; which is now wrong
    for powerpc64.
    
    Fix the value by explicit define the previous glibc constants
    (powerpc64 does not need to use the 32 kABI value, but it simplifies
    the FCNTL_ADJUST_CMD which should be kept as compatibility).
    
    Checked on powerpc64-linux-gnu and powerpc-linux-gnu.

This commit correctly fixes the issue for the case where _FILE_OFFSET_BITS is not defined, but introduce a new issue for the case -D_FILE_OFFSET_BITS=64, causing an ABI breakage. This can be observed for instance in perl and libfile-fcntllock-perl if they are not built against the same glibc version.

Note that the patch has been backported to at least the 2.36 and 2.37 branches.
Comment 2 Aurelien Jarno 2023-08-28 21:44:06 UTC
Proposed patch to fix the issue: https://sourceware.org/pipermail/libc-alpha/2023-August/151199.html
Comment 3 Andreas Schwab 2023-08-29 08:07:22 UTC
Note that the kernel ABI for ppc64 only uses the 5/6/7 values, and the glibc fcntl wrapper accepts both ranges, so this is not an ABI break.
Comment 4 Florian Weimer 2023-08-29 08:37:36 UTC
It's the historic behavior. With glibc-2.17-326.el7_9.ppc64:

#include <fcntl.h>
#include <stdio.h>

int
main (void)
{
#ifdef _FILE_OFFSET_BITS
  printf ("_FILE_OFFSET_BITS=%d\n", _FILE_OFFSET_BITS);
#endif
  printf ("F_GETLK=%d\n", F_GETLK);
  printf ("F_SETLK=%d\n", F_SETLK);
  printf ("F_SETLKW=%d\n", F_SETLKW);
  return 0;
}

This gives:

F_GETLK=5
F_SETLK=6
F_SETLKW=7

Or:

_FILE_OFFSET_BITS=64
F_GETLK=12
F_SETLK=13
F_SETLKW=14

glibc-2.17-326.el7_9.ppc64le shows the same thing.
Comment 5 Adhemerval Zanella 2023-08-29 13:42:37 UTC
The powercp64 fcntl will handle F_GETLK/F_SETLK/F_SETLKW with the historical values with the FCNTL_ADJUST_CMD macro, so old binaries will continue to work as expected. I am not sure if we still need to keep exporting old values for powerpc64.
Comment 6 Florian Weimer 2023-08-29 14:01:19 UTC
(In reply to Adhemerval Zanella from comment #5)
> The powercp64 fcntl will handle F_GETLK/F_SETLK/F_SETLKW with the historical
> values with the FCNTL_ADJUST_CMD macro, so old binaries will continue to
> work as expected. I am not sure if we still need to keep exporting old
> values for powerpc64.

Agreed, my comment was misguided, I thought the proposed change was going in the opposite direction. I do not think we need to preserve the old behavior.
Comment 7 Sourceware Commits 2023-09-07 19:57:11 UTC
The master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=434bf72a94de68f0cc7fbf3c44bf38c1911b70cb

commit 434bf72a94de68f0cc7fbf3c44bf38c1911b70cb
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Mon Aug 28 23:30:37 2023 +0200

    io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64
    
    Commit 5f828ff824e3b7cd1 ("io: Fix F_GETLK, F_SETLK, and F_SETLKW for
    powerpc64") fixed an issue with the value of the lock constants on
    powerpc64 when not using __USE_FILE_OFFSET64, but it ended-up also
    changing the value when using __USE_FILE_OFFSET64 causing an API change.
    
    Fix that by also checking that define, restoring the pre
    4d0fe291aed3a476a commit values:
    
    Default values:
    - F_GETLK: 5
    - F_SETLK: 6
    - F_SETLKW: 7
    
    With -D_FILE_OFFSET_BITS=64:
    - F_GETLK: 12
    - F_SETLK: 13
    - F_SETLKW: 14
    
    At the same time, it has been noticed that there was no test for io lock
    with __USE_FILE_OFFSET64, so just add one.
    
    Tested on x86_64-linux-gnu, i686-linux-gnu and
    powerpc64le-unknown-linux-gnu.
    
    Resolves: BZ #30804.
    Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Comment 8 Aurelien Jarno 2023-09-07 19:59:26 UTC
Fixed on 2.39
Comment 9 Sourceware Commits 2023-09-08 05:28:04 UTC
The release/2.38/master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5bdef6f27c91f45505ed5444147be4ed0e9bc3c7

commit 5bdef6f27c91f45505ed5444147be4ed0e9bc3c7
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Mon Aug 28 23:30:37 2023 +0200

    io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64
    
    Commit 5f828ff824e3b7cd1 ("io: Fix F_GETLK, F_SETLK, and F_SETLKW for
    powerpc64") fixed an issue with the value of the lock constants on
    powerpc64 when not using __USE_FILE_OFFSET64, but it ended-up also
    changing the value when using __USE_FILE_OFFSET64 causing an API change.
    
    Fix that by also checking that define, restoring the pre
    4d0fe291aed3a476a commit values:
    
    Default values:
    - F_GETLK: 5
    - F_SETLK: 6
    - F_SETLKW: 7
    
    With -D_FILE_OFFSET_BITS=64:
    - F_GETLK: 12
    - F_SETLK: 13
    - F_SETLKW: 14
    
    At the same time, it has been noticed that there was no test for io lock
    with __USE_FILE_OFFSET64, so just add one.
    
    Tested on x86_64-linux-gnu, i686-linux-gnu and
    powerpc64le-unknown-linux-gnu.
    
    Resolves: BZ #30804.
    Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
    (cherry picked from commit 434bf72a94de68f0cc7fbf3c44bf38c1911b70cb)
Comment 10 Sourceware Commits 2023-09-08 05:28:34 UTC
The release/2.37/master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=be26b29262bbae080acb8bb16855df6ac4c57c98

commit be26b29262bbae080acb8bb16855df6ac4c57c98
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Mon Aug 28 23:30:37 2023 +0200

    io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64
    
    Commit 5f828ff824e3b7cd1 ("io: Fix F_GETLK, F_SETLK, and F_SETLKW for
    powerpc64") fixed an issue with the value of the lock constants on
    powerpc64 when not using __USE_FILE_OFFSET64, but it ended-up also
    changing the value when using __USE_FILE_OFFSET64 causing an API change.
    
    Fix that by also checking that define, restoring the pre
    4d0fe291aed3a476a commit values:
    
    Default values:
    - F_GETLK: 5
    - F_SETLK: 6
    - F_SETLKW: 7
    
    With -D_FILE_OFFSET_BITS=64:
    - F_GETLK: 12
    - F_SETLK: 13
    - F_SETLKW: 14
    
    At the same time, it has been noticed that there was no test for io lock
    with __USE_FILE_OFFSET64, so just add one.
    
    Tested on x86_64-linux-gnu, i686-linux-gnu and
    powerpc64le-unknown-linux-gnu.
    
    Resolves: BZ #30804.
    Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
    (cherry picked from commit 434bf72a94de68f0cc7fbf3c44bf38c1911b70cb)
Comment 11 Sourceware Commits 2023-09-15 21:20:53 UTC
The release/2.36/master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0ae2afe45ee0e88b37b89399b6a8cf0330f46937

commit 0ae2afe45ee0e88b37b89399b6a8cf0330f46937
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Mon Aug 28 23:30:37 2023 +0200

    io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64
    
    Commit 5f828ff824e3b7cd1 ("io: Fix F_GETLK, F_SETLK, and F_SETLKW for
    powerpc64") fixed an issue with the value of the lock constants on
    powerpc64 when not using __USE_FILE_OFFSET64, but it ended-up also
    changing the value when using __USE_FILE_OFFSET64 causing an API change.
    
    Fix that by also checking that define, restoring the pre
    4d0fe291aed3a476a commit values:
    
    Default values:
    - F_GETLK: 5
    - F_SETLK: 6
    - F_SETLKW: 7
    
    With -D_FILE_OFFSET_BITS=64:
    - F_GETLK: 12
    - F_SETLK: 13
    - F_SETLKW: 14
    
    At the same time, it has been noticed that there was no test for io lock
    with __USE_FILE_OFFSET64, so just add one.
    
    Tested on x86_64-linux-gnu, i686-linux-gnu and
    powerpc64le-unknown-linux-gnu.
    
    Resolves: BZ #30804.
    Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
    (cherry picked from commit 434bf72a94de68f0cc7fbf3c44bf38c1911b70cb)
Comment 12 Sourceware Commits 2023-09-16 09:11:52 UTC
The release/2.35/master branch has been updated by Aurelien Jarno <aurel32@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=762a747faefad05cbfa463a1e7f7b00684d56f77

commit 762a747faefad05cbfa463a1e7f7b00684d56f77
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Mon Aug 28 23:30:37 2023 +0200

    io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64
    
    Commit 5f828ff824e3b7cd1 ("io: Fix F_GETLK, F_SETLK, and F_SETLKW for
    powerpc64") fixed an issue with the value of the lock constants on
    powerpc64 when not using __USE_FILE_OFFSET64, but it ended-up also
    changing the value when using __USE_FILE_OFFSET64 causing an API change.
    
    Fix that by also checking that define, restoring the pre
    4d0fe291aed3a476a commit values:
    
    Default values:
    - F_GETLK: 5
    - F_SETLK: 6
    - F_SETLKW: 7
    
    With -D_FILE_OFFSET_BITS=64:
    - F_GETLK: 12
    - F_SETLK: 13
    - F_SETLKW: 14
    
    At the same time, it has been noticed that there was no test for io lock
    with __USE_FILE_OFFSET64, so just add one.
    
    Tested on x86_64-linux-gnu, i686-linux-gnu and
    powerpc64le-unknown-linux-gnu.
    
    Resolves: BZ #30804.
    Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
    (cherry picked from commit 434bf72a94de68f0cc7fbf3c44bf38c1911b70cb)