[RFC PATCH ghak10 v2 4/4] [OPTIONAL] Determine read-only in the validation function

[Ondrej Mosnacek]

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