2018-06-16 18:44 GMT+02:00 Richard Guy Briggs <rgb(a)redhat.com>:
On 2018-06-15 14:45, Ondrej Mosnacek wrote:
> (Intentionally not sending to the timekeeping/ntp maintainers just yet,
> let's settle on the record contents/format first.)
>
> Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
> ---
> kernel/time/ntp.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index a09ded765f6c..78a01df0dbdb 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/rtc.h>
> #include <linux/math64.h>
> +#include <linux/audit.h>
>
> #include "ntp_internal.h"
> #include "timekeeping_internal.h"
> @@ -722,6 +723,16 @@ int __do_adjtimex(struct timex *txc, struct timespec64 *ts, s32
*time_tai)
> {
> int result;
>
> + /* Only log audit event if the clock was changed/attempted to be changed.
> + * Based on the logic inside timekeeping_validate_timex().
> + * NOTE: We need to log the event before any of the fields get
> + * overwritten by the output values (the function will not fail, so it
> + * is OK). */
> + if ( ( (txc->modes & ADJ_ADJTIME) && !(txc->modes &
ADJ_OFFSET_READONLY))
> + || (!(txc->modes & ADJ_ADJTIME) && txc->modes)
> + || (txc->modes & ADJ_SETOFFSET))
Wouldn't the third condition be covered by the second?
Not if txc->modes has ADJ_ADJTIME set, ADJ_OFFSET_READONLY unset, and
ADJ_SETOFFSET set. Then it fails the first two conditions and only
matches on the third one. I don't know if such combination can
actually occur in practice, but timekeeping_validate_timex() doesn't
reject it, so it's better to be on the safe side here.
BTW, I just realized that the logging function can be called directly
from inside the do_adjtimex() function in timekeeping.c (which is a
wrapper around __do_adjtimex()), where it probably suits better (since
this function contains the ADJ_SETOFFSET handling code and
timekeeping.c also contains the timekeeping_validate_timex() function
referenced in the comment). I will move it in v2.
> + audit_adjtime(txc);
> +
> if (txc->modes & ADJ_ADJTIME) {
> long save_adjust = time_adjust;
- RGB
--
Richard Guy Briggs <rgb(a)redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.