time: Fix overwrite err unexpected in clock_adjtime32
PATCH 1/1 time: Fix overwrite err unexpected in clock_adjtime32
the correct error is covered by put_old_timex32.
--- kernel/time/posix-timers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/time/posix-timers.c +2 -2
@@ -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; }
Signature · 1 line
2.9.4
Quoted 1 line · Chen Jun
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().
Quoted 1 line
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
Quoted 23 lines
Signed-off-by Chen Jun <[email protected]>--- kernel/time/posix-timers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)kernel/time/posix-timers.c +2
-2@@ -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; }Signature · 1 line
2.9.4
Quoted 27 lines · Richard Cochran
Quoted 1 line · Chen Jun
the correct error is covered by
put_old_timex32.Well, the non-negative return code (
TIME_OK,TIME_INS, etc) is clobbered byput_old_timex32().Quoted 1 line
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
f1f1d5ebd10fthe 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);
Quoted 9 lines
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.
Quoted 29 lines
Thanks,
RichardQuoted 23 lines
Signed-off-by Chen Jun <[email protected]>--- kernel/time/posix-timers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)kernel/time/posix-timers.c +2
-2@@ -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; }Signature · 1 line
2.9.4
Signature · 2 lines
Regards Chen Jun
Quoted 30 lines · chenjun (AM)
Quoted 29 lines · Richard Cochran
Quoted 1 line · Chen Jun
the correct error is covered by
put_old_timex32.Well, the non-negative return code (
TIME_OK,TIME_INS, etc) is clobbered byput_old_timex32().Quoted 1 line
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
f1f1d5ebd10fthe 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);
Quoted 6 lines
Quoted 6 lines
err = compat_put_timex(utp, &txc); if (err) return err; return ret; }
Look at the code ^^^
Quoted 2 lines
The implement of
clock_adjtime32is similar tocompat_sys_clock_adjtime. And I thinkf1f1d5ebd10introduced the problem actually.
See how 'ret' and 'err' are two separate variables? It makes a difference.
Thanks,
Richard
Quoted 40 lines · Richard Cochran
Quoted 30 lines · chenjun (AM)
Quoted 29 lines · Richard Cochran
Quoted 1 line · Chen Jun
the correct error is covered by
put_old_timex32.Well, the non-negative return code (
TIME_OK,TIME_INS, etc) is clobbered byput_old_timex32().Quoted 1 line
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
f1f1d5ebd10fthe 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);Quoted 6 lines
Quoted 6 lines
err = compat_put_timex(utp, &txc); if (err) return err; return ret; }Look at the code ^^^
Quoted 8 lines
Quoted 2 lines
The implement of
clock_adjtime32is similar tocompat_sys_clock_adjtime. And I thinkf1f1d5ebd10introduced 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.
Signature · 2 lines
Regards Chen Jun