| Message ID | 20220623165226.1335679-1-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | timekeeping: contribute wall clock to rng on time change | expand |
On Thu, Jun 23, 2022 at 06:52:26PM +0200, Jason A. Donenfeld wrote: > The rng's random_init() function contributes the real time to the rng at > boot time, so that events can at least start in relation to something > particular in the real world. But this clock might not yet be set that > point in boot, so nothing is contributed. In addition, the relation > between minor clock changes from, say, NTP, and the cycle counter is > potentially useful entropic data. > > This commit addresses this by mixing in a time stamp on calls to > settimeofday and adjtimex. No entropy is credited in doing so, so it > doesn't make initialization faster, but it is still useful input to > have. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Cc: stable@vger.kernel.org > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Good idea. > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 8e4b3c32fcf9..ad55da792f13 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c This doesn't compile: kernel/time/timekeeping.c: In function ‘do_settimeofday64’: kernel/time/timekeeping.c:1350:9: error: implicit declaration of function ‘add_device_randomness’ [-Werror=implicit-function-declaratio ] 1350 | add_device_randomness(&xt, sizeof(xt)); | ^~~~~~~~~~~~~~~~~~~~~ > @@ -1346,6 +1346,9 @@ int do_settimeofday64(const struct timespec64 *ts) > if (!ret) > audit_tk_injoffset(ts_delta); > > + ktime_get_real_ts64(&xt); > + add_device_randomness(&xt, sizeof(xt)); > + > return ret; Isn't the new time already available in 'ts'? Is the call to ktime_get_real_ts64() necessary? > } > EXPORT_SYMBOL(do_settimeofday64); > @@ -2475,6 +2478,9 @@ int do_adjtimex(struct __kernel_timex *txc) > > ntp_notify_cmos_timer(); > > + ktime_get_real_ts64(&ts); > + add_device_randomness(&ts, sizeof(ts)); > + > return ret; > } adjtimex() actually triggers a gradual adjustment of the clock, rather than setting it immediately. Is there a way to mix in the target time rather than the current time as this does? - Eric
Hi Eric, On Thu, Jun 23, 2022 at 7:39 PM Eric Biggers <ebiggers@kernel.org> wrote: > This doesn't compile: Doh. I had the missing include, but missed it in `git add -p`. Will be fixed for v2. > > @@ -1346,6 +1346,9 @@ int do_settimeofday64(const struct timespec64 *ts) > > if (!ret) > > audit_tk_injoffset(ts_delta); > > > > + ktime_get_real_ts64(&xt); > > + add_device_randomness(&xt, sizeof(xt)); > > + > > return ret; > > Isn't the new time already available in 'ts'? Is the call to > ktime_get_real_ts64() necessary? Good point; you're right. Will simplify as such. > > ntp_notify_cmos_timer(); > > > > + ktime_get_real_ts64(&ts); > > + add_device_randomness(&ts, sizeof(ts)); > > + > > return ret; > > } > > adjtimex() actually triggers a gradual adjustment of the clock, rather than > setting it immediately. Is there a way to mix in the target time rather than > the current time as this does? Hmm... I have no idea. But maybe instead I can just call `add_device_randomness()` on that big struct of offsets and things. If the kernel is able to use it to set the time, then probably it's a sufficient encoding of that end state. Jason
Hi "Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/timers/core]
[also build test ERROR on linus/master v5.19-rc3 next-20220623]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jason-A-Donenfeld/timekeeping-contribute-wall-clock-to-rng-on-time-change/20220624-010017
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 57963a92a70b037aa22544fbc34742e5be689c04
config: i386-tinyconfig (https://download.01.org/0day-ci/archive/20220624/202206240128.jVs1F5jD-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f2f11bc49b9a71c5663e44d46c8265f9b4fc8011
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jason-A-Donenfeld/timekeeping-contribute-wall-clock-to-rng-on-time-change/20220624-010017
git checkout f2f11bc49b9a71c5663e44d46c8265f9b4fc8011
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/time/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/time/timekeeping.c: In function 'do_settimeofday64':
>> kernel/time/timekeeping.c:1349:9: error: implicit declaration of function 'add_device_randomness' [-Werror=implicit-function-declaration]
1349 | add_device_randomness(&xt, sizeof(xt));
| ^~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/add_device_randomness +1349 kernel/time/timekeeping.c
1303
1304 /**
1305 * do_settimeofday64 - Sets the time of day.
1306 * @ts: pointer to the timespec64 variable containing the new time
1307 *
1308 * Sets the time of day to the new time and update NTP and notify hrtimers
1309 */
1310 int do_settimeofday64(const struct timespec64 *ts)
1311 {
1312 struct timekeeper *tk = &tk_core.timekeeper;
1313 struct timespec64 ts_delta, xt;
1314 unsigned long flags;
1315 int ret = 0;
1316
1317 if (!timespec64_valid_settod(ts))
1318 return -EINVAL;
1319
1320 raw_spin_lock_irqsave(&timekeeper_lock, flags);
1321 write_seqcount_begin(&tk_core.seq);
1322
1323 timekeeping_forward_now(tk);
1324
1325 xt = tk_xtime(tk);
1326 ts_delta = timespec64_sub(*ts, xt);
1327
1328 if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {
1329 ret = -EINVAL;
1330 goto out;
1331 }
1332
1333 tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
1334
1335 tk_set_xtime(tk, ts);
1336 out:
1337 timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
1338
1339 write_seqcount_end(&tk_core.seq);
1340 raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
1341
1342 /* Signal hrtimers about time change */
1343 clock_was_set(CLOCK_SET_WALL);
1344
1345 if (!ret)
1346 audit_tk_injoffset(ts_delta);
1347
1348 ktime_get_real_ts64(&xt);
> 1349 add_device_randomness(&xt, sizeof(xt));
1350
1351 return ret;
1352 }
1353 EXPORT_SYMBOL(do_settimeofday64);
1354
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 8e4b3c32fcf9..ad55da792f13 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1346,6 +1346,9 @@ int do_settimeofday64(const struct timespec64 *ts) if (!ret) audit_tk_injoffset(ts_delta); + ktime_get_real_ts64(&xt); + add_device_randomness(&xt, sizeof(xt)); + return ret; } EXPORT_SYMBOL(do_settimeofday64); @@ -2475,6 +2478,9 @@ int do_adjtimex(struct __kernel_timex *txc) ntp_notify_cmos_timer(); + ktime_get_real_ts64(&ts); + add_device_randomness(&ts, sizeof(ts)); + return ret; }
The rng's random_init() function contributes the real time to the rng at boot time, so that events can at least start in relation to something particular in the real world. But this clock might not yet be set that point in boot, so nothing is contributed. In addition, the relation between minor clock changes from, say, NTP, and the cycle counter is potentially useful entropic data. This commit addresses this by mixing in a time stamp on calls to settimeofday and adjtimex. No entropy is credited in doing so, so it doesn't make initialization faster, but it is still useful input to have. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- kernel/time/timekeeping.c | 6 ++++++ 1 file changed, 6 insertions(+)