* [PATCH] time: Fix overwrite err unexpected in clock_adjtime32
@ 2021-04-12 12:45 Chen Jun
2021-04-12 14:19 ` Richard Cochran
0 siblings, 1 reply; 5+ messages in thread
From: Chen Jun @ 2021-04-12 12:45 UTC (permalink / raw)
To: linux-kernel; +Cc: tglx, richardcochran, johnstul, rui.xiang
the correct error is covered by put_old_timex32.
Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.")
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
kernel/time/posix-timers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index bf540f5a..dd5697d 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1191,8 +1191,8 @@ SYSCALL_DEFINE2(clock_adjtime32, clockid_t, which_clock,
err = do_clock_adjtime(which_clock, &ktx);
- if (err >= 0)
- err = put_old_timex32(utp, &ktx);
+ if (err >= 0 && put_old_timex32(utp, &ktx))
+ return -EFAULT;
return err;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 2021-04-12 12:45 [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 Chen Jun @ 2021-04-12 14:19 ` Richard Cochran 2021-04-12 14:52 ` chenjun (AM) 0 siblings, 1 reply; 5+ messages in thread From: Richard Cochran @ 2021-04-12 14:19 UTC (permalink / raw) To: Chen Jun; +Cc: linux-kernel, tglx, johnstul, rui.xiang On Mon, Apr 12, 2021 at 12:45:51PM +0000, Chen Jun wrote: > the correct error is covered by put_old_timex32. Well, the non-negative return code (TIME_OK, TIME_INS, etc) is clobbered by put_old_timex32(). > Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.") This is not the correct commit for the "Fixes" tag. Please find the actual commit that introduced the issue. In commit f1f1d5ebd10f the code looked like this... long compat_sys_clock_adjtime(clockid_t which_clock, struct compat_timex __user *utp) { struct timex txc; mm_segment_t oldfs; int err, ret; err = compat_get_timex(&txc, utp); if (err) return err; oldfs = get_fs(); set_fs(KERNEL_DS); ret = sys_clock_adjtime(which_clock, (struct timex __user *) &txc); set_fs(oldfs); err = compat_put_timex(utp, &txc); if (err) return err; return ret; } Thanks, Richard > Signed-off-by: Chen Jun <chenjun102@huawei.com> > --- > kernel/time/posix-timers.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c > index bf540f5a..dd5697d 100644 > --- a/kernel/time/posix-timers.c > +++ b/kernel/time/posix-timers.c > @@ -1191,8 +1191,8 @@ SYSCALL_DEFINE2(clock_adjtime32, clockid_t, which_clock, > > err = do_clock_adjtime(which_clock, &ktx); > > - if (err >= 0) > - err = put_old_timex32(utp, &ktx); > + if (err >= 0 && put_old_timex32(utp, &ktx)) > + return -EFAULT; > > return err; > } > -- > 2.9.4 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 2021-04-12 14:19 ` Richard Cochran @ 2021-04-12 14:52 ` chenjun (AM) 2021-04-12 15:58 ` Richard Cochran 0 siblings, 1 reply; 5+ messages in thread From: chenjun (AM) @ 2021-04-12 14:52 UTC (permalink / raw) To: Richard Cochran; +Cc: linux-kernel, tglx, johnstul, Xiangrui (Euler) 在 2021/4/12 22:20, Richard Cochran 写道: > On Mon, Apr 12, 2021 at 12:45:51PM +0000, Chen Jun wrote: >> the correct error is covered by put_old_timex32. > > Well, the non-negative return code (TIME_OK, TIME_INS, etc) is > clobbered by put_old_timex32(). > >> Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.") > > This is not the correct commit for the "Fixes" tag. Please find the > actual commit that introduced the issue. > > In commit f1f1d5ebd10f the code looked like this... > > long compat_sys_clock_adjtime(clockid_t which_clock, > struct compat_timex __user *utp) > { > struct timex txc; > mm_segment_t oldfs; > int err, ret; > > err = compat_get_timex(&txc, utp); > if (err) > return err; > > oldfs = get_fs(); > set_fs(KERNEL_DS); > ret = sys_clock_adjtime(which_clock, (struct timex __user *) &txc); > set_fs(oldfs); > > err = compat_put_timex(utp, &txc); > if (err) > return err; > > return ret; > } > f1f1d5ebd10: Introduce compat_sys_clock_adjtime 62a6fa97684: rename compat_sys_clock_adjtime to COMPAT_SYSCALL_DEFINE2(clock_adjtime 3a4d44b6162: move COMPAT_SYSCALL_DEFINE2(clock_adjtime from kernel/compat.c to kernel/time/posix-timers.c 8dabe7245bb: COMPAT_SYSCALL_DEFINE2(clock_adjtime, .. -> SYSCALL_DEFINE2(clock_adjtime32, .. The implement of clock_adjtime32 is similar to compat_sys_clock_adjtime. And I think f1f1d5ebd10 introduced the problem actually. > Thanks, > Richard > > > >> Signed-off-by: Chen Jun <chenjun102@huawei.com> >> --- >> kernel/time/posix-timers.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c >> index bf540f5a..dd5697d 100644 >> --- a/kernel/time/posix-timers.c >> +++ b/kernel/time/posix-timers.c >> @@ -1191,8 +1191,8 @@ SYSCALL_DEFINE2(clock_adjtime32, clockid_t, which_clock, >> >> err = do_clock_adjtime(which_clock, &ktx); >> >> - if (err >= 0) >> - err = put_old_timex32(utp, &ktx); >> + if (err >= 0 && put_old_timex32(utp, &ktx)) >> + return -EFAULT; >> >> return err; >> } >> -- >> 2.9.4 >> > -- Regards Chen Jun ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 2021-04-12 14:52 ` chenjun (AM) @ 2021-04-12 15:58 ` Richard Cochran 2021-04-13 1:47 ` chenjun (AM) 0 siblings, 1 reply; 5+ messages in thread From: Richard Cochran @ 2021-04-12 15:58 UTC (permalink / raw) To: chenjun (AM); +Cc: linux-kernel, tglx, john.stultz, Xiangrui (Euler) On Mon, Apr 12, 2021 at 02:52:11PM +0000, chenjun (AM) wrote: > 在 2021/4/12 22:20, Richard Cochran 写道: > > On Mon, Apr 12, 2021 at 12:45:51PM +0000, Chen Jun wrote: > >> the correct error is covered by put_old_timex32. > > > > Well, the non-negative return code (TIME_OK, TIME_INS, etc) is > > clobbered by put_old_timex32(). > > > >> Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.") > > > > This is not the correct commit for the "Fixes" tag. Please find the > > actual commit that introduced the issue. > > > > In commit f1f1d5ebd10f the code looked like this... > > > > long compat_sys_clock_adjtime(clockid_t which_clock, > > struct compat_timex __user *utp) > > { > > struct timex txc; > > mm_segment_t oldfs; > > int err, ret; > > > > err = compat_get_timex(&txc, utp); > > if (err) > > return err; > > > > oldfs = get_fs(); > > set_fs(KERNEL_DS); > > ret = sys_clock_adjtime(which_clock, (struct timex __user *) &txc); > > set_fs(oldfs); > > > > err = compat_put_timex(utp, &txc); > > if (err) > > return err; > > > > return ret; > > } Look at the code ^^^ > The implement of clock_adjtime32 is similar to compat_sys_clock_adjtime. > And I think f1f1d5ebd10 introduced the problem actually. See how 'ret' and 'err' are two separate variables? It makes a difference. Thanks, Richard ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 2021-04-12 15:58 ` Richard Cochran @ 2021-04-13 1:47 ` chenjun (AM) 0 siblings, 0 replies; 5+ messages in thread From: chenjun (AM) @ 2021-04-13 1:47 UTC (permalink / raw) To: Richard Cochran; +Cc: linux-kernel, tglx, john.stultz, Xiangrui (Euler) 在 2021/4/12 23:58, Richard Cochran 写道: > On Mon, Apr 12, 2021 at 02:52:11PM +0000, chenjun (AM) wrote: >> 在 2021/4/12 22:20, Richard Cochran 写道: >>> On Mon, Apr 12, 2021 at 12:45:51PM +0000, Chen Jun wrote: >>>> the correct error is covered by put_old_timex32. >>> >>> Well, the non-negative return code (TIME_OK, TIME_INS, etc) is >>> clobbered by put_old_timex32(). >>> >>>> Fixes: f1f1d5ebd10f ("posix-timers: Introduce a syscall for clock tuning.") >>> >>> This is not the correct commit for the "Fixes" tag. Please find the >>> actual commit that introduced the issue. >>> >>> In commit f1f1d5ebd10f the code looked like this... >>> >>> long compat_sys_clock_adjtime(clockid_t which_clock, >>> struct compat_timex __user *utp) >>> { >>> struct timex txc; >>> mm_segment_t oldfs; >>> int err, ret; >>> >>> err = compat_get_timex(&txc, utp); >>> if (err) >>> return err; >>> >>> oldfs = get_fs(); >>> set_fs(KERNEL_DS); >>> ret = sys_clock_adjtime(which_clock, (struct timex __user *) &txc); >>> set_fs(oldfs); >>> >>> err = compat_put_timex(utp, &txc); >>> if (err) >>> return err; >>> >>> return ret; >>> } > > Look at the code ^^^ > >> The implement of clock_adjtime32 is similar to compat_sys_clock_adjtime. >> And I think f1f1d5ebd10 introduced the problem actually. > > See how 'ret' and 'err' are two separate variables? It makes a difference. > > Thanks, > Richard > Oh, yee.. Very thanks. 3a4d44b616 ("ntp: Move adjtimex related compat syscalls to native counterparts") made the change. -- Regards Chen Jun ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-13 1:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-12 12:45 [PATCH] time: Fix overwrite err unexpected in clock_adjtime32 Chen Jun 2021-04-12 14:19 ` Richard Cochran 2021-04-12 14:52 ` chenjun (AM) 2021-04-12 15:58 ` Richard Cochran 2021-04-13 1:47 ` chenjun (AM)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).