| Message ID | 20250507-fixes-modify_ldt-v1-1-70a2694cfddc@suse.com |
|---|---|
| State | Needs Review / ACK |
| Headers | show |
| Series | ldt.h: Add workaround for x86_64 | expand |
| Context | Check | Description |
|---|---|---|
| ltpci/debian_stable_s390x-linux-gnu-gcc_s390x | success | success |
| ltpci/debian_stable_aarch64-linux-gnu-gcc_arm64 | success | success |
| ltpci/debian_stable_powerpc64le-linux-gnu-gcc_ppc64el | success | success |
| ltpci/debian_stable_gcc | success | success |
| ltpci/ubuntu_bionic_gcc | success | success |
| ltpci/debian_stable_gcc | success | success |
| ltpci/quay-io-centos-centos_stream9_gcc | success | success |
| ltpci/ubuntu_jammy_gcc | success | success |
| ltpci/opensuse-leap_latest_gcc | success | success |
| ltpci/debian_testing_clang | success | success |
| ltpci/debian_testing_gcc | success | success |
| ltpci/debian_oldstable_clang | success | success |
| ltpci/opensuse-archive_42-2_gcc | success | success |
| ltpci/alpine_latest_gcc | success | success |
| ltpci/fedora_latest_clang | success | success |
| ltpci/debian_oldstable_gcc | success | success |
Hi, one issue below. On 08. 05. 25 1:37, Ricardo B. Marlière wrote: > From: Ricardo B. Marlière <rbm@suse.com> > > The commit be0aaca2f742 ("syscalls/modify_ldt: Add lapi/ldt.h") left behind > an important factor of modify_ldt(): the kernel intentionally casts the > return value to unsigned int. This was handled in > testcases/cve/cve-2015-3290.c but was removed. Add it back to the relevant > file. > > Reported-by: Martin Doucha <mdoucha@suse.cz> > Signed-off-by: Ricardo B. Marlière <rbm@suse.com> > --- > include/lapi/ldt.h | 22 +++++++++++++++++++++- > testcases/cve/cve-2015-3290.c | 6 +++++- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/include/lapi/ldt.h b/include/lapi/ldt.h > index 6b5a2d59cb41bfc24eb5ac26c3d47d49fb8ff78f..173321dd9ac34ba87eff0eee960635f30d878991 100644 > --- a/include/lapi/ldt.h > +++ b/include/lapi/ldt.h > @@ -31,7 +31,27 @@ struct user_desc { > static inline int modify_ldt(int func, const struct user_desc *ptr, > unsigned long bytecount) > { > - return tst_syscall(__NR_modify_ldt, func, ptr, bytecount); > + long rval; > + > + errno = 0; > + rval = tst_syscall(__NR_modify_ldt, func, ptr, bytecount); > + > +#ifdef __x86_64__ > + /* > + * The kernel intentionally casts modify_ldt() return value > + * to unsigned int to prevent sign extension to 64 bits. This may > + * result in syscall() returning the value as is instead of setting > + * errno and returning -1. > + */ > + if (rval > 0 && (int)rval < 0) { > + tst_res(TINFO, > + "WARNING: Libc mishandled modify_ldt() return value"); > + errno = -(int)errno; > + rval = -1; > + } > +#endif /* __x86_64__ */ > + > + return rval; > } > > static inline int safe_modify_ldt(const char *file, const int lineno, int func, > diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c > index 8ec1d53bbb5a9f3e7761d39855d34f593e118a28..6aa064bab30a039d268b2e9f17258564eda8067c 100644 > --- a/testcases/cve/cve-2015-3290.c > +++ b/testcases/cve/cve-2015-3290.c > @@ -197,7 +197,11 @@ static void set_ldt(void) > .useable = 0 > }; > > - SAFE_MODIFY_LDT(1, &data_desc, sizeof(data_desc)); > + TEST(modify_ldt(1, &data_desc, sizeof(data_desc))); > + if (TST_RET == -1 && TST_ERR == EINVAL) { > + tst_brk(TCONF | TTERRNO, > + "modify_ldt: 16-bit data segments are probably disabled"); > + } This part is correct, but any other non-zero return value is an error that should trigger TBROK. See the old test code before refactoring. > } > > static void try_corrupt_stack(unsigned short *orig_ss) > > --- > base-commit: b070a5692e035ec12c3d3c7a7e9e97c270fd4d7d > change-id: 20250507-fixes-modify_ldt-ebcfdd2a7d30 > > Best regards,
diff --git a/include/lapi/ldt.h b/include/lapi/ldt.h index 6b5a2d59cb41bfc24eb5ac26c3d47d49fb8ff78f..173321dd9ac34ba87eff0eee960635f30d878991 100644 --- a/include/lapi/ldt.h +++ b/include/lapi/ldt.h @@ -31,7 +31,27 @@ struct user_desc { static inline int modify_ldt(int func, const struct user_desc *ptr, unsigned long bytecount) { - return tst_syscall(__NR_modify_ldt, func, ptr, bytecount); + long rval; + + errno = 0; + rval = tst_syscall(__NR_modify_ldt, func, ptr, bytecount); + +#ifdef __x86_64__ + /* + * The kernel intentionally casts modify_ldt() return value + * to unsigned int to prevent sign extension to 64 bits. This may + * result in syscall() returning the value as is instead of setting + * errno and returning -1. + */ + if (rval > 0 && (int)rval < 0) { + tst_res(TINFO, + "WARNING: Libc mishandled modify_ldt() return value"); + errno = -(int)errno; + rval = -1; + } +#endif /* __x86_64__ */ + + return rval; } static inline int safe_modify_ldt(const char *file, const int lineno, int func, diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c index 8ec1d53bbb5a9f3e7761d39855d34f593e118a28..6aa064bab30a039d268b2e9f17258564eda8067c 100644 --- a/testcases/cve/cve-2015-3290.c +++ b/testcases/cve/cve-2015-3290.c @@ -197,7 +197,11 @@ static void set_ldt(void) .useable = 0 }; - SAFE_MODIFY_LDT(1, &data_desc, sizeof(data_desc)); + TEST(modify_ldt(1, &data_desc, sizeof(data_desc))); + if (TST_RET == -1 && TST_ERR == EINVAL) { + tst_brk(TCONF | TTERRNO, + "modify_ldt: 16-bit data segments are probably disabled"); + } } static void try_corrupt_stack(unsigned short *orig_ss)