* [PATCH] debugfs: convert gid= argument from decimal, not octal
@ 2013-01-02 13:54 Dave Reisner
2013-01-02 18:33 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Dave Reisner @ 2013-01-02 13:54 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Dave Reisner
This patch technically breaks userspace, but I suspect that anyone who
actually used this flag would have encountered this brokenness, declared
it lunacy, and already sent a patch.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
fs/debugfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 153bb1e..a5f12b7 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -176,7 +176,7 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
opts->uid = uid;
break;
case Opt_gid:
- if (match_octal(&args[0], &option))
+ if (match_int(&args[0], &option))
return -EINVAL;
gid = make_kgid(current_user_ns(), option);
if (!gid_valid(gid))
--
1.8.0.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] debugfs: convert gid= argument from decimal, not octal
2013-01-02 13:54 [PATCH] debugfs: convert gid= argument from decimal, not octal Dave Reisner
@ 2013-01-02 18:33 ` Greg Kroah-Hartman
2013-01-02 18:42 ` Kees Cook
2013-01-02 18:46 ` Dave Reisner
0 siblings, 2 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2013-01-02 18:33 UTC (permalink / raw)
To: Dave Reisner, keescook; +Cc: linux-kernel
On Wed, Jan 02, 2013 at 08:54:37AM -0500, Dave Reisner wrote:
> This patch technically breaks userspace, but I suspect that anyone who
> actually used this flag would have encountered this brokenness, declared
> it lunacy, and already sent a patch.
Kees is the one that originally did this change (I think, right?)
Kees, any objection to this patch? Will it break your existing systems?
thanks,
greg k-h
>
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
> fs/debugfs/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index 153bb1e..a5f12b7 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -176,7 +176,7 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
> opts->uid = uid;
> break;
> case Opt_gid:
> - if (match_octal(&args[0], &option))
> + if (match_int(&args[0], &option))
> return -EINVAL;
> gid = make_kgid(current_user_ns(), option);
> if (!gid_valid(gid))
> --
> 1.8.0.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] debugfs: convert gid= argument from decimal, not octal
2013-01-02 18:33 ` Greg Kroah-Hartman
@ 2013-01-02 18:42 ` Kees Cook
2013-01-02 18:57 ` Greg Kroah-Hartman
2013-01-02 19:06 ` [kernel-hardening] " Vasiliy Kulikov
2013-01-02 18:46 ` Dave Reisner
1 sibling, 2 replies; 6+ messages in thread
From: Kees Cook @ 2013-01-02 18:42 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Dave Reisner, LKML, Vasiliy Kulikov
On Wed, Jan 2, 2013 at 10:33 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Jan 02, 2013 at 08:54:37AM -0500, Dave Reisner wrote:
>> This patch technically breaks userspace, but I suspect that anyone who
>> actually used this flag would have encountered this brokenness, declared
>> it lunacy, and already sent a patch.
>
> Kees is the one that originally did this change (I think, right?)
I recommended the default change, but I think it was Vasiliy that
added the gid option, IIRC.
> Kees, any objection to this patch? Will it break your existing systems?
Regardless, I have no objection -- this should be int, not octal.
-Kees
>
> thanks,
>
> greg k-h
>
>>
>> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
>> ---
>> fs/debugfs/inode.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
>> index 153bb1e..a5f12b7 100644
>> --- a/fs/debugfs/inode.c
>> +++ b/fs/debugfs/inode.c
>> @@ -176,7 +176,7 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
>> opts->uid = uid;
>> break;
>> case Opt_gid:
>> - if (match_octal(&args[0], &option))
>> + if (match_int(&args[0], &option))
>> return -EINVAL;
>> gid = make_kgid(current_user_ns(), option);
>> if (!gid_valid(gid))
>> --
>> 1.8.0.3
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] debugfs: convert gid= argument from decimal, not octal
2013-01-02 18:33 ` Greg Kroah-Hartman
2013-01-02 18:42 ` Kees Cook
@ 2013-01-02 18:46 ` Dave Reisner
1 sibling, 0 replies; 6+ messages in thread
From: Dave Reisner @ 2013-01-02 18:46 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Dave Reisner, keescook, linux-kernel, ludwig.nussel
On Wed, Jan 02, 2013 at 10:33:51AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 02, 2013 at 08:54:37AM -0500, Dave Reisner wrote:
> > This patch technically breaks userspace, but I suspect that anyone who
> > actually used this flag would have encountered this brokenness, declared
> > it lunacy, and already sent a patch.
>
> Kees is the one that originally did this change (I think, right?)
>
> Kees, any objection to this patch? Will it break your existing systems?
>
> thanks,
>
> greg k-h
>
Actually the original patch was provided by Ludwig Nussel (cc'd) and
commited as d6e486868cde58. Sorry, I should have mentioned at least the
commit that introduced this in the original mailing.
d
> >
> > Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> > ---
> > fs/debugfs/inode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> > index 153bb1e..a5f12b7 100644
> > --- a/fs/debugfs/inode.c
> > +++ b/fs/debugfs/inode.c
> > @@ -176,7 +176,7 @@ static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
> > opts->uid = uid;
> > break;
> > case Opt_gid:
> > - if (match_octal(&args[0], &option))
> > + if (match_int(&args[0], &option))
> > return -EINVAL;
> > gid = make_kgid(current_user_ns(), option);
> > if (!gid_valid(gid))
> > --
> > 1.8.0.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] debugfs: convert gid= argument from decimal, not octal
2013-01-02 18:42 ` Kees Cook
@ 2013-01-02 18:57 ` Greg Kroah-Hartman
2013-01-02 19:06 ` [kernel-hardening] " Vasiliy Kulikov
1 sibling, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2013-01-02 18:57 UTC (permalink / raw)
To: Kees Cook; +Cc: Dave Reisner, LKML, Vasiliy Kulikov
On Wed, Jan 02, 2013 at 10:42:17AM -0800, Kees Cook wrote:
> On Wed, Jan 2, 2013 at 10:33 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Wed, Jan 02, 2013 at 08:54:37AM -0500, Dave Reisner wrote:
> >> This patch technically breaks userspace, but I suspect that anyone who
> >> actually used this flag would have encountered this brokenness, declared
> >> it lunacy, and already sent a patch.
> >
> > Kees is the one that originally did this change (I think, right?)
>
> I recommended the default change, but I think it was Vasiliy that
> added the gid option, IIRC.
Ah, you are right, sorry about that.
> > Kees, any objection to this patch? Will it break your existing systems?
>
> Regardless, I have no objection -- this should be int, not octal.
Ok, I'll queue this up when I get back from vacation next week.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kernel-hardening] Re: [PATCH] debugfs: convert gid= argument from decimal, not octal
2013-01-02 18:42 ` Kees Cook
2013-01-02 18:57 ` Greg Kroah-Hartman
@ 2013-01-02 19:06 ` Vasiliy Kulikov
1 sibling, 0 replies; 6+ messages in thread
From: Vasiliy Kulikov @ 2013-01-02 19:06 UTC (permalink / raw)
To: Kees Cook; +Cc: Greg Kroah-Hartman, Dave Reisner, LKML, kernel-hardening
Hi,
(CC'ed kernel-hardening ML)
On Wed, Jan 02, 2013 at 10:42 -0800, Kees Cook wrote:
> On Wed, Jan 2, 2013 at 10:33 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Wed, Jan 02, 2013 at 08:54:37AM -0500, Dave Reisner wrote:
> >> This patch technically breaks userspace, but I suspect that anyone who
> >> actually used this flag would have encountered this brokenness, declared
> >> it lunacy, and already sent a patch.
> >
> > Kees is the one that originally did this change (I think, right?)
>
> I recommended the default change, but I think it was Vasiliy that
> added the gid option, IIRC.
Wow. It was not me :-) IIRC, I haven't proposed debugfs uid/gid stuff
after Greg NACK'ed my sysfs uid/gid/umask mount options patch. IIRC,
there were talks that debugfs must not be mounted on production systems
because it exists for debug reasons only, so no additional security
settings are needed.
My version of the patch (which was not posted on LKML):
http://www.openwall.com/lists/kernel-hardening/2011/06/05/4 (FWIW, it doesn't contain the bug in question.)
>
> > Kees, any objection to this patch? Will it break your existing systems?
>
> Regardless, I have no objection -- this should be int, not octal.
No objection. GID should be decimal.
> -Kees
>
> >
> > thanks,
> >
> > greg k-h
> >
> >>
> >> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
Reviewed-by: Vasiliy Kulikov <segoon@openwall.com>
> >> ---
> >> fs/debugfs/inode.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks,
--
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-02 19:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-02 13:54 [PATCH] debugfs: convert gid= argument from decimal, not octal Dave Reisner
2013-01-02 18:33 ` Greg Kroah-Hartman
2013-01-02 18:42 ` Kees Cook
2013-01-02 18:57 ` Greg Kroah-Hartman
2013-01-02 19:06 ` [kernel-hardening] " Vasiliy Kulikov
2013-01-02 18:46 ` Dave Reisner
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.