Skip to contentSkip to conversation

ADJ_OFFSET_SS_READ and capabilities

PATCH In review Michael Kerrisk 3 participants 4 messages Jun 23, 2008 – Jul 21, 2008 1 message not in the archive Timekeeping, clocksource core, NTP, alarmtimer

lore
Share & tools
lore

Michael Kerrisk

author
#1 lore raw

PATCH 1/1 ADJ_OFFSET_SS_READ and capabilities

In reply to a message not in the archive

Hi Roman, John,

ADJ_OFFSET_SS_READ is a read-only operation. Therefore, it seems reasonable not to require any capability (as is the case when 'modes' is zero. See the patch below. Does this change seem reasonable?

Cheers,

Michael

kernel/time/ntp.c +2 -1
@@ -281,7 +281,8 @@
 	int result;
 
 	/* In order to modify anything, you gotta be super-user! */
-	if (txc->modes && !capable(CAP_SYS_TIME))
+	if (txc->modes && txc->modes != ADJ_OFFSET_SS_READ &&
+	    !capable(CAP_SYS_TIME))
 		return -EPERM;
 
 	/* Now we validate the data before disabling interrupts */
author
#4 lore raw

PATCH 1/1 adjtimex() modes argument checking

In reply to a message not in the archive

Hi Roman,

I see you added a number of new modes to adtimex() in 2.6.26-rc. Since these are userspace-visible changes, please CC me, so that they stand a chance of getting documented in man-pages. (I discovered these changes only by accident.)

I also have some changes to suggest for argument checking, which, if I understand the code correctly, prevent obvious screw-ups by callers of adjtimex(). What do you think of the patch below?

Cheers,

Michael

kernel/time/ntp.c 2008-06-22 09:04:31.000000000 +0200 renamed from kernel/time/ntp.c 2008-06-22 09:05:05.000000000 +0200 +10 -0
@@ -292,6 +292,16 @@
                        return -EINVAL;
        }
 
+       /* These modes are the converse of one another  */
+
+       if ((txc->modes & ADJ_MICRO) && (txc->modes & ADJ_NANO))
+               return -EINVAL;
+
+       /* Both of the following want to use txc->constant */
+
+       if ((txc->modes & ADJ_TIMECONST) && (txc->modes & ADJ_TAI))
+               return -EINVAL;
+
        /* if the quartz is off by more than 10% something is VERY wrong ! */
        if (txc->modes & ADJ_TICK)
                if (txc->tick <  900000/USER_HZ ||
#2 lore raw
In reply to Michael Kerrisk
Quoted 14 lines · Michael Kerrisk

Hi Roman, John,

ADJ_OFFSET_SS_READ is a read-only operation. Therefore, it seems reasonable not to require any capability (as is the case when 'modes' is zero. See the patch below. Does this change seem reasonable?

Cheers,

Michael

kernel/time/ntp.c line 288

Quoted 9 lines
kernel/time/ntp.c +2 -1
@@ -281,7 +281,8 @@
  	int result;
 
  	/* In order to modify anything, you gotta be super-user! */
-	if (txc->modes && !capable(CAP_SYS_TIME))
+	if (txc->modes && txc->modes != ADJ_OFFSET_SS_READ &&
+	    !capable(CAP_SYS_TIME))
  		return -EPERM;
 
  	/* Now we validate the data before disabling interrupts */

Hey Michael,
This seems like an ok change, but we'd first want to fix the issue you
pointed out earlier which would make sure adjtimex() read calls don't cause side effects.

thanks
-john

#3 lore raw
Quoted 23 lines · john stultz
Quoted 21 lines · Michael Kerrisk

Hi Roman, John,

ADJ_OFFSET_SS_READ is a read-only operation. Therefore, it seems reasonable not to require any capability (as is the case when 'modes' is zero. See the patch below. Does this change seem reasonable?

Cheers,

Michael

kernel/time/ntp.c 2008-06-22 07:31:43.000000000 +0200 renamed from kernel/time/ntp.c 2008-06-13 11:16:51.000000000 +0200 +2 -1
@@ -281,7 +281,8 @@
      int result;
 
      /* In order to modify anything, you gotta be super-user! */
-     if (txc->modes && !capable(CAP_SYS_TIME))
+     if (txc->modes && txc->modes != ADJ_OFFSET_SS_READ &&
+         !capable(CAP_SYS_TIME))
              return -EPERM;
Quoted 8 lines

kernel/time/ntp.c 2008-06-22 07:31:43.000000000 +0200 line 288

Quoted 2 lines
kernel/time/ntp.c 2008-06-22 07:31:43.000000000 +0200 renamed from kernel/time/ntp.c 2008-06-13 11:16:51.000000000 +0200 +0 -0
      /* Now we validate the data before disabling interrupts */

Hey Michael,
This seems like an ok change, but we'd first want to fix the issue you
pointed out earlier which would make sure adjtimex() read calls don't cause side effects.

John, Roman,

Are you pushing this into 2.6.27-rc1?

Cheers,

Michael

Signature · 4 lines
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html