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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michael Kerrisk

author

2008-06-23 08:02 UTC #1 lore raw

PATCH 1/1 ADJ_OFFSET_SS_READ and capabilities

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 */

john stultz

2008-06-30 22:08 UTC #2 lore raw

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

Michael Kerrisk

2008-07-21 10:38 UTC #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

Michael Kerrisk

author

2008-06-23 08:18 UTC #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 ||