From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ondrej Mosnacek Subject: [RFC PATCH ghak10 v2 4/4] [OPTIONAL] Determine read-only in the validation function Date: Tue, 19 Jun 2018 15:59:01 +0200 Message-ID: <20180619135901.31473-5-omosnace@redhat.com> References: <20180619135901.31473-1-omosnace@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (ext-mx12.extmail.prod.ext.phx2.redhat.com [10.5.110.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A7CF31001914 for ; Tue, 19 Jun 2018 13:59:43 +0000 (UTC) Received: from mail-wm0-f72.google.com (mail-wm0-f72.google.com [74.125.82.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 96695308A96B for ; Tue, 19 Jun 2018 13:59:43 +0000 (UTC) Received: by mail-wm0-f72.google.com with SMTP id 76-v6so227583wmw.3 for ; Tue, 19 Jun 2018 06:59:43 -0700 (PDT) In-Reply-To: <20180619135901.31473-1-omosnace@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com Cc: Richard Guy Briggs List-Id: linux-audit@redhat.com 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