All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] timerfd add flags check
@ 2009-02-09 23:31 Davide Libenzi
  2009-02-10  0:36 ` Michael Kerrisk
  0 siblings, 1 reply; 4+ messages in thread
From: Davide Libenzi @ 2009-02-09 23:31 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: Michael Kerrisk, Greg KH, Andrew Morton

Like Michael requested, this patch adds a missing check for valid flags in 
timerfd_settime(), and make it return EINVAL in case some extra bits are 
set.

Michael said:
If this is to be any use to userland apps that want to check flag
support (perhaps it is too late already), then the sooner we get it
into the kernel the better: 2.6.29 would be good; earlier stables as
well would be even better.

Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>


Signed-off-by: Davide Libenzi <davidel@xmailserver.org>


- Davide


---
 fs/timerfd.c            |   12 ++++++------
 include/linux/timerfd.h |   17 ++++++++++++++---
 2 files changed, 20 insertions(+), 9 deletions(-)

Index: linux-2.6.mod/fs/timerfd.c
===================================================================
--- linux-2.6.mod.orig/fs/timerfd.c	2009-02-08 18:36:45.000000000 -0800
+++ linux-2.6.mod/fs/timerfd.c	2009-02-08 18:53:32.000000000 -0800
@@ -186,10 +186,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
 	BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
 	BUILD_BUG_ON(TFD_NONBLOCK != O_NONBLOCK);
 
-	if (flags & ~(TFD_CLOEXEC | TFD_NONBLOCK))
-		return -EINVAL;
-	if (clockid != CLOCK_MONOTONIC &&
-	    clockid != CLOCK_REALTIME)
+	if ((flags & ~TFD_CREATE_FLAGS) ||
+	    (clockid != CLOCK_MONOTONIC &&
+	     clockid != CLOCK_REALTIME))
 		return -EINVAL;
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -201,7 +200,7 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
 	hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS);
 
 	ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
-			       flags & (O_CLOEXEC | O_NONBLOCK));
+			       flags & TFD_SHARED_FCNTL_FLAGS);
 	if (ufd < 0)
 		kfree(ctx);
 
@@ -219,7 +218,8 @@ SYSCALL_DEFINE4(timerfd_settime, int, uf
 	if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
 		return -EFAULT;
 
-	if (!timespec_valid(&ktmr.it_value) ||
+	if ((flags & ~TFD_SETTIME_FLAGS) ||
+	    !timespec_valid(&ktmr.it_value) ||
 	    !timespec_valid(&ktmr.it_interval))
 		return -EINVAL;
 
Index: linux-2.6.mod/include/linux/timerfd.h
===================================================================
--- linux-2.6.mod.orig/include/linux/timerfd.h	2009-02-08 18:36:45.000000000 -0800
+++ linux-2.6.mod/include/linux/timerfd.h	2009-02-08 18:41:22.000000000 -0800
@@ -11,13 +11,24 @@
 /* For O_CLOEXEC and O_NONBLOCK */
 #include <linux/fcntl.h>
 
-/* Flags for timerfd_settime.  */
+/*
+ * CAREFUL: Check include/asm-generic/fcntl.h when defining
+ * new flags, since they might collide with O_* ones. We want
+ * to re-use O_* flags that couldn't possibly have a meaning
+ * from eventfd, in order to leave a free define-space for
+ * shared O_* flags.
+ */
 #define TFD_TIMER_ABSTIME (1 << 0)
-
-/* Flags for timerfd_create.  */
 #define TFD_CLOEXEC O_CLOEXEC
 #define TFD_NONBLOCK O_NONBLOCK
 
+#define TFD_SHARED_FCNTL_FLAGS (TFD_CLOEXEC | TFD_NONBLOCK)
+/* Flags for timerfd_create.  */
+#define TFD_CREATE_FLAGS TFD_SHARED_FCNTL_FLAGS
+/* Flags for timerfd_settime.  */
+#define TFD_SETTIME_FLAGS TFD_TIMER_ABSTIME
+#define TFD_FLAGS_SET (TFD_SHARED_FCNTL_FLAGS | TFD_TIMER_ABSTIME)
+
 
 #endif /* _LINUX_TIMERFD_H */
 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [patch] timerfd add flags check
  2009-02-09 23:31 [patch] timerfd add flags check Davide Libenzi
@ 2009-02-10  0:36 ` Michael Kerrisk
  2009-02-11  4:42   ` Michael Kerrisk
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Kerrisk @ 2009-02-10  0:36 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Linux Kernel Mailing List, Greg KH, Andrew Morton

On Tue, Feb 10, 2009 at 12:31 PM, Davide Libenzi
<davidel@xmailserver.org> wrote:
> Like Michael requested, this patch adds a missing check for valid flags in
> timerfd_settime(), and make it return EINVAL in case some extra bits are
> set.
>
> Michael said:
> If this is to be any use to userland apps that want to check flag
> support (perhaps it is too late already), then the sooner we get it
> into the kernel the better: 2.6.29 would be good; earlier stables as
> well would be even better.
>
> Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
>
>
> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>

Davide, one question: is the TFD_FLAGS_SET constant below needed?

cheers,

Michael

> ---
>  fs/timerfd.c            |   12 ++++++------
>  include/linux/timerfd.h |   17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.mod/fs/timerfd.c
> ===================================================================
> --- linux-2.6.mod.orig/fs/timerfd.c     2009-02-08 18:36:45.000000000 -0800
> +++ linux-2.6.mod/fs/timerfd.c  2009-02-08 18:53:32.000000000 -0800
> @@ -186,10 +186,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
>        BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
>        BUILD_BUG_ON(TFD_NONBLOCK != O_NONBLOCK);
>
> -       if (flags & ~(TFD_CLOEXEC | TFD_NONBLOCK))
> -               return -EINVAL;
> -       if (clockid != CLOCK_MONOTONIC &&
> -           clockid != CLOCK_REALTIME)
> +       if ((flags & ~TFD_CREATE_FLAGS) ||
> +           (clockid != CLOCK_MONOTONIC &&
> +            clockid != CLOCK_REALTIME))
>                return -EINVAL;
>
>        ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -201,7 +200,7 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
>        hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS);
>
>        ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
> -                              flags & (O_CLOEXEC | O_NONBLOCK));
> +                              flags & TFD_SHARED_FCNTL_FLAGS);
>        if (ufd < 0)
>                kfree(ctx);
>
> @@ -219,7 +218,8 @@ SYSCALL_DEFINE4(timerfd_settime, int, uf
>        if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
>                return -EFAULT;
>
> -       if (!timespec_valid(&ktmr.it_value) ||
> +       if ((flags & ~TFD_SETTIME_FLAGS) ||
> +           !timespec_valid(&ktmr.it_value) ||
>            !timespec_valid(&ktmr.it_interval))
>                return -EINVAL;
>
> Index: linux-2.6.mod/include/linux/timerfd.h
> ===================================================================
> --- linux-2.6.mod.orig/include/linux/timerfd.h  2009-02-08 18:36:45.000000000 -0800
> +++ linux-2.6.mod/include/linux/timerfd.h       2009-02-08 18:41:22.000000000 -0800
> @@ -11,13 +11,24 @@
>  /* For O_CLOEXEC and O_NONBLOCK */
>  #include <linux/fcntl.h>
>
> -/* Flags for timerfd_settime.  */
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
>  #define TFD_TIMER_ABSTIME (1 << 0)
> -
> -/* Flags for timerfd_create.  */
>  #define TFD_CLOEXEC O_CLOEXEC
>  #define TFD_NONBLOCK O_NONBLOCK
>
> +#define TFD_SHARED_FCNTL_FLAGS (TFD_CLOEXEC | TFD_NONBLOCK)
> +/* Flags for timerfd_create.  */
> +#define TFD_CREATE_FLAGS TFD_SHARED_FCNTL_FLAGS
> +/* Flags for timerfd_settime.  */
> +#define TFD_SETTIME_FLAGS TFD_TIMER_ABSTIME
> +#define TFD_FLAGS_SET (TFD_SHARED_FCNTL_FLAGS | TFD_TIMER_ABSTIME)
> +
>
>  #endif /* _LINUX_TIMERFD_H */
>
>



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [patch] timerfd add flags check
  2009-02-10  0:36 ` Michael Kerrisk
@ 2009-02-11  4:42   ` Michael Kerrisk
  2009-02-11 19:35     ` Davide Libenzi
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Kerrisk @ 2009-02-11  4:42 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Linux Kernel Mailing List, Greg KH, Andrew Morton

On Tue, Feb 10, 2009 at 1:36 PM, Michael Kerrisk
<mtk.manpages@googlemail.com> wrote:
> On Tue, Feb 10, 2009 at 12:31 PM, Davide Libenzi
> <davidel@xmailserver.org> wrote:
>> Like Michael requested, this patch adds a missing check for valid flags in
>> timerfd_settime(), and make it return EINVAL in case some extra bits are
>> set.
>>
>> Michael said:
>> If this is to be any use to userland apps that want to check flag
>> support (perhaps it is too late already), then the sooner we get it
>> into the kernel the better: 2.6.29 would be good; earlier stables as
>> well would be even better.
>>
>> Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
>>
>>
>> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
>
> Davide, one question: is the TFD_FLAGS_SET constant below needed?

Davide, Ping!  I see Andrew already took the patch, which seems to
define this unnecessary macro.

Cheers,

Michael

>
>> ---
>>  fs/timerfd.c            |   12 ++++++------
>>  include/linux/timerfd.h |   17 ++++++++++++++---
>>  2 files changed, 20 insertions(+), 9 deletions(-)
>>
>> Index: linux-2.6.mod/fs/timerfd.c
>> ===================================================================
>> --- linux-2.6.mod.orig/fs/timerfd.c     2009-02-08 18:36:45.000000000 -0800
>> +++ linux-2.6.mod/fs/timerfd.c  2009-02-08 18:53:32.000000000 -0800
>> @@ -186,10 +186,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
>>        BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
>>        BUILD_BUG_ON(TFD_NONBLOCK != O_NONBLOCK);
>>
>> -       if (flags & ~(TFD_CLOEXEC | TFD_NONBLOCK))
>> -               return -EINVAL;
>> -       if (clockid != CLOCK_MONOTONIC &&
>> -           clockid != CLOCK_REALTIME)
>> +       if ((flags & ~TFD_CREATE_FLAGS) ||
>> +           (clockid != CLOCK_MONOTONIC &&
>> +            clockid != CLOCK_REALTIME))
>>                return -EINVAL;
>>
>>        ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> @@ -201,7 +200,7 @@ SYSCALL_DEFINE2(timerfd_create, int, clo
>>        hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS);
>>
>>        ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
>> -                              flags & (O_CLOEXEC | O_NONBLOCK));
>> +                              flags & TFD_SHARED_FCNTL_FLAGS);
>>        if (ufd < 0)
>>                kfree(ctx);
>>
>> @@ -219,7 +218,8 @@ SYSCALL_DEFINE4(timerfd_settime, int, uf
>>        if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
>>                return -EFAULT;
>>
>> -       if (!timespec_valid(&ktmr.it_value) ||
>> +       if ((flags & ~TFD_SETTIME_FLAGS) ||
>> +           !timespec_valid(&ktmr.it_value) ||
>>            !timespec_valid(&ktmr.it_interval))
>>                return -EINVAL;
>>
>> Index: linux-2.6.mod/include/linux/timerfd.h
>> ===================================================================
>> --- linux-2.6.mod.orig/include/linux/timerfd.h  2009-02-08 18:36:45.000000000 -0800
>> +++ linux-2.6.mod/include/linux/timerfd.h       2009-02-08 18:41:22.000000000 -0800
>> @@ -11,13 +11,24 @@
>>  /* For O_CLOEXEC and O_NONBLOCK */
>>  #include <linux/fcntl.h>
>>
>> -/* Flags for timerfd_settime.  */
>> +/*
>> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> + * new flags, since they might collide with O_* ones. We want
>> + * to re-use O_* flags that couldn't possibly have a meaning
>> + * from eventfd, in order to leave a free define-space for
>> + * shared O_* flags.
>> + */
>>  #define TFD_TIMER_ABSTIME (1 << 0)
>> -
>> -/* Flags for timerfd_create.  */
>>  #define TFD_CLOEXEC O_CLOEXEC
>>  #define TFD_NONBLOCK O_NONBLOCK
>>
>> +#define TFD_SHARED_FCNTL_FLAGS (TFD_CLOEXEC | TFD_NONBLOCK)
>> +/* Flags for timerfd_create.  */
>> +#define TFD_CREATE_FLAGS TFD_SHARED_FCNTL_FLAGS
>> +/* Flags for timerfd_settime.  */
>> +#define TFD_SETTIME_FLAGS TFD_TIMER_ABSTIME
>> +#define TFD_FLAGS_SET (TFD_SHARED_FCNTL_FLAGS | TFD_TIMER_ABSTIME)
>> +
>>
>>  #endif /* _LINUX_TIMERFD_H */
>>
>>
>
>
>
> --
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
> 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
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [patch] timerfd add flags check
  2009-02-11  4:42   ` Michael Kerrisk
@ 2009-02-11 19:35     ` Davide Libenzi
  0 siblings, 0 replies; 4+ messages in thread
From: Davide Libenzi @ 2009-02-11 19:35 UTC (permalink / raw)
  To: mtk.manpages; +Cc: Linux Kernel Mailing List, Greg KH, Andrew Morton

On Wed, 11 Feb 2009, Michael Kerrisk wrote:

> On Tue, Feb 10, 2009 at 1:36 PM, Michael Kerrisk
> <mtk.manpages@googlemail.com> wrote:
> > On Tue, Feb 10, 2009 at 12:31 PM, Davide Libenzi
> > <davidel@xmailserver.org> wrote:
> >> Like Michael requested, this patch adds a missing check for valid flags in
> >> timerfd_settime(), and make it return EINVAL in case some extra bits are
> >> set.
> >>
> >> Michael said:
> >> If this is to be any use to userland apps that want to check flag
> >> support (perhaps it is too late already), then the sooner we get it
> >> into the kernel the better: 2.6.29 would be good; earlier stables as
> >> well would be even better.
> >>
> >> Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
> >>
> >>
> >> Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
> >
> > Davide, one question: is the TFD_FLAGS_SET constant below needed?
> 
> Davide, Ping!  I see Andrew already took the patch, which seems to
> define this unnecessary macro.

Yes, Michael! I will post a correction to Andrew. OTOH an un-necessary 
macro defined could even be passed into the lower priority queue, don't 
you think?



- Davide



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-02-11 19:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-09 23:31 [patch] timerfd add flags check Davide Libenzi
2009-02-10  0:36 ` Michael Kerrisk
2009-02-11  4:42   ` Michael Kerrisk
2009-02-11 19:35     ` Davide Libenzi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.