This is an optional patch that modifies the previous one to determine
whether the operation is read-only in the timekeeping_validate_timex()
function instead of duplicating the complex condition.

A minor issue with this patch is that the validation function now
sometimes returns -EINVAL where it would before return -EPERM in the
cases where both errors are applicable. This may not be desired.

Please ACK or NACK, I'm not sure which variant is better...
---
 kernel/time/timekeeping.c | 40 ++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 02097a7d225e..dad11205a86d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2220,20 +2220,25 @@ ktime_t ktime_get_update_offsets_now(unsigned int 
*cwsseq, ktime_t *offs_real,
 
 /**
  * timekeeping_validate_timex - Ensures the timex is ok for use in do_adjtimex
+ *
+ * Return value:
+ *   1 - OK, syscall is read-only
+ *   0 - OK, syscall modifies some value
+ *   negative - NOT OK (-EINVAL or -EPERM)
  */
 static int timekeeping_validate_timex(struct timex *txc)
 {
+       int readonly = 1;
+
        if (txc->modes & ADJ_ADJTIME) {
                /* singleshot must not be used with any other mode bits */
                if (!(txc->modes & ADJ_OFFSET_SINGLESHOT))
                        return -EINVAL;
-               if (!(txc->modes & ADJ_OFFSET_READONLY) &&
-                   !capable(CAP_SYS_TIME))
-                       return -EPERM;
+               if (!(txc->modes & ADJ_OFFSET_READONLY))
+                       readonly = 0;
        } else {
-               /* In order to modify anything, you gotta be super-user! */
-               if (txc->modes && !capable(CAP_SYS_TIME))
-                       return -EPERM;
+               if (txc->modes)
+                       readonly = 0;
                /*
                 * if the quartz is off by more than 10% then
                 * something is VERY wrong!
@@ -2245,9 +2250,7 @@ static int timekeeping_validate_timex(struct timex *txc)
        }
 
        if (txc->modes & ADJ_SETOFFSET) {
-               /* In order to inject time, you gotta be super-user! */
-               if (!capable(CAP_SYS_TIME))
-                       return -EPERM;
+               readonly = 0;
 
                /*
                 * Validate if a timespec/timeval used to inject a time
@@ -2269,6 +2272,10 @@ static int timekeeping_validate_timex(struct timex *txc)
                }
        }
 
+       /* In order to modify anything, you gotta be super-user! */
+       if (!readonly && !capable(CAP_SYS_TIME))
+               return -EPERM;
+
        /*
         * Check for potential multiplication overflows that can
         * only happen on 64-bit systems:
@@ -2280,7 +2287,7 @@ static int timekeeping_validate_timex(struct timex *txc)
                        return -EINVAL;
        }
 
-       return 0;
+       return readonly;
 }
 
 
@@ -2293,13 +2300,15 @@ int do_adjtimex(struct timex *txc)
        unsigned long flags;
        struct timespec64 ts;
        s32 orig_tai, tai;
-       int ret;
+       int ret, readonly;
 
        /* Validate the data before disabling interrupts */
        ret = timekeeping_validate_timex(txc);
-       if (ret)
+       if (ret < 0)
                return ret;
 
+       readonly = ret;
+
        if (txc->modes & ADJ_SETOFFSET) {
                struct timespec64 delta;
                delta.tv_sec  = txc->time.tv_sec;
@@ -2316,14 +2325,11 @@ int do_adjtimex(struct timex *txc)
        raw_spin_lock_irqsave(&timekeeper_lock, flags);
        write_seqcount_begin(&tk_core.seq);
 
-       /* Only log audit event if the clock was changed/attempted to be 
changed.
-        * Based on the logic inside timekeeping_validate_timex().
+       /* Only log audit event if the clock is being changed.
         * NOTE: We need to log the event before any of the fields get
         * overwritten by the output values (__do_adjtimex() does not fail, so
         * it is OK to do it here). */
-       if (   ( (txc->modes & ADJ_ADJTIME) && !(txc->modes & 
ADJ_OFFSET_READONLY))
-           || (!(txc->modes & ADJ_ADJTIME) &&   txc->modes)
-           ||   (txc->modes & ADJ_SETOFFSET))
+       if (!readonly)
                audit_adjtime(txc);
 
        orig_tai = tai = tk->tai_offset;
-- 
2.17.1

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to