[RFC PATCH ghak10 v2 4/4] [OPTIONAL] Determine read-only in the validation function
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