All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Joe Perches <joe@perches.com>,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	kernel-hardening@lists.openwall.com
Subject: Re: [PATCH] video: constify geode ops structures
Date: Mon, 9 Nov 2015 06:39:43 +0000 (GMT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1511090634180.2593@hadrien> (raw)
In-Reply-To: <20151109054253.GQ18797@mwanda>



On Mon, 9 Nov 2015, Dan Carpenter wrote:

> On Sun, Nov 08, 2015 at 10:24:49PM +0000, Julia Lawall wrote:
> > On Mon, 9 Nov 2015, Dan Carpenter wrote:
> >
> > > Cool.  So, in grsec they use a GCC plugin to make these const
> > > automatically since they only contain function pointers.  There about
> > > 100 struct types marked as __no_const.  Kees would like to adopt the
> > > grsec pluggin approach I expect.  Do you have an idea how many structs
> > > only contain function pointers or how many consts we would have to add
> > > to get the same effect without the plugin?
> >
> > My list has 373 type names.  In the list there are counts for good
> > (already const) and bad (not const).  The sum of the bad values is 2467.
> > The list is below.
> >
> > julia
>
> Fantastic!  Thanks.  We could autogenerate the list of type names and
> make checkpatch.pl complain if we declared those types as non const.
>
> I ran this command to find which functions grsec marks as __no_const.
>
> egrep '(^ struct |^@@|__no_const;)' grsecurity-3.1-4.2.5-201511021814.patch | grep __no_const -B1 | grep -v __no_const | grep -v '^--' | cut -d @ -f 5-  | cut -b 9- | cut -d ' ' -f 1
>
> There are 60 structs declared as __no_const.  For some structs they
> declare a no_const version and use it as needed.  Like this:
> typedef struct net_device_ops __no_const net_device_ops_no_const;
>
> grep __no_const grsecurity-3.1-4.2.5-201511021814.patch | grep typedef | cut -d ' ' -f 3
>
> There are 32 of those.
>
> Then I compared to see if their structs were on your list.  For some
> reason there quite a few one their list which are not on yours.  Out
> of the first 10 about half weren't on your list.  cpu_cache_fns,
> outer_cache_fns, psci_operations, smp_operations, omap_hwmod_soc_ops,
> smp_ops_t.  These are mostly different arches?
>
> Also bit_table has in int has well as a function pointers but it is on
> their list.  I'm not sure why.  Maybe they are marking structs const
> that I don't know about.
>
> The other trick that they do is they define structs as __do_const if
> they want them to be const by default, which is pretty neat.  This feels
> like it should be a standard GCC feature.  In the meantime we could
> mark things as __do_const and print a sparse warning if it was declared
> as not const.
>
> I have attached the list of __no_const structs.

Thanks.  Architectures wouldn't matter for me, but an adjacent parsing
problem or a strangely written type name could cause a problem.  I will
check your list.

I would think though that the real problem wuld be things like the
platform_driver structure, which to my recollection has one non-constant
field, so the structure can't be const.

julia

WARNING: multiple messages have this Message-ID (diff)
From: Julia Lawall <julia.lawall@lip6.fr>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Joe Perches <joe@perches.com>,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	kernel-hardening@lists.openwall.com
Subject: Re: [PATCH] video: constify geode ops structures
Date: Mon, 09 Nov 2015 06:39:43 +0000	[thread overview]
Message-ID: <alpine.DEB.2.10.1511090634180.2593@hadrien> (raw)
In-Reply-To: <20151109054253.GQ18797@mwanda>



On Mon, 9 Nov 2015, Dan Carpenter wrote:

> On Sun, Nov 08, 2015 at 10:24:49PM +0000, Julia Lawall wrote:
> > On Mon, 9 Nov 2015, Dan Carpenter wrote:
> >
> > > Cool.  So, in grsec they use a GCC plugin to make these const
> > > automatically since they only contain function pointers.  There about
> > > 100 struct types marked as __no_const.  Kees would like to adopt the
> > > grsec pluggin approach I expect.  Do you have an idea how many structs
> > > only contain function pointers or how many consts we would have to add
> > > to get the same effect without the plugin?
> >
> > My list has 373 type names.  In the list there are counts for good
> > (already const) and bad (not const).  The sum of the bad values is 2467.
> > The list is below.
> >
> > julia
>
> Fantastic!  Thanks.  We could autogenerate the list of type names and
> make checkpatch.pl complain if we declared those types as non const.
>
> I ran this command to find which functions grsec marks as __no_const.
>
> egrep '(^ struct |^@@|__no_const;)' grsecurity-3.1-4.2.5-201511021814.patch | grep __no_const -B1 | grep -v __no_const | grep -v '^--' | cut -d @ -f 5-  | cut -b 9- | cut -d ' ' -f 1
>
> There are 60 structs declared as __no_const.  For some structs they
> declare a no_const version and use it as needed.  Like this:
> typedef struct net_device_ops __no_const net_device_ops_no_const;
>
> grep __no_const grsecurity-3.1-4.2.5-201511021814.patch | grep typedef | cut -d ' ' -f 3
>
> There are 32 of those.
>
> Then I compared to see if their structs were on your list.  For some
> reason there quite a few one their list which are not on yours.  Out
> of the first 10 about half weren't on your list.  cpu_cache_fns,
> outer_cache_fns, psci_operations, smp_operations, omap_hwmod_soc_ops,
> smp_ops_t.  These are mostly different arches?
>
> Also bit_table has in int has well as a function pointers but it is on
> their list.  I'm not sure why.  Maybe they are marking structs const
> that I don't know about.
>
> The other trick that they do is they define structs as __do_const if
> they want them to be const by default, which is pretty neat.  This feels
> like it should be a standard GCC feature.  In the meantime we could
> mark things as __do_const and print a sparse warning if it was declared
> as not const.
>
> I have attached the list of __no_const structs.

Thanks.  Architectures wouldn't matter for me, but an adjacent parsing
problem or a strangely written type name could cause a problem.  I will
check your list.

I would think though that the real problem wuld be things like the
platform_driver structure, which to my recollection has one non-constant
field, so the structure can't be const.

julia

WARNING: multiple messages have this Message-ID (diff)
From: Julia Lawall <julia.lawall@lip6.fr>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Joe Perches <joe@perches.com>,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	kernel-hardening@lists.openwall.com
Subject: [kernel-hardening] Re: [PATCH] video: constify geode ops structures
Date: Mon, 9 Nov 2015 06:39:43 +0000 (GMT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1511090634180.2593@hadrien> (raw)
In-Reply-To: <20151109054253.GQ18797@mwanda>



On Mon, 9 Nov 2015, Dan Carpenter wrote:

> On Sun, Nov 08, 2015 at 10:24:49PM +0000, Julia Lawall wrote:
> > On Mon, 9 Nov 2015, Dan Carpenter wrote:
> >
> > > Cool.  So, in grsec they use a GCC plugin to make these const
> > > automatically since they only contain function pointers.  There about
> > > 100 struct types marked as __no_const.  Kees would like to adopt the
> > > grsec pluggin approach I expect.  Do you have an idea how many structs
> > > only contain function pointers or how many consts we would have to add
> > > to get the same effect without the plugin?
> >
> > My list has 373 type names.  In the list there are counts for good
> > (already const) and bad (not const).  The sum of the bad values is 2467.
> > The list is below.
> >
> > julia
>
> Fantastic!  Thanks.  We could autogenerate the list of type names and
> make checkpatch.pl complain if we declared those types as non const.
>
> I ran this command to find which functions grsec marks as __no_const.
>
> egrep '(^ struct |^@@|__no_const;)' grsecurity-3.1-4.2.5-201511021814.patch | grep __no_const -B1 | grep -v __no_const | grep -v '^--' | cut -d @ -f 5-  | cut -b 9- | cut -d ' ' -f 1
>
> There are 60 structs declared as __no_const.  For some structs they
> declare a no_const version and use it as needed.  Like this:
> typedef struct net_device_ops __no_const net_device_ops_no_const;
>
> grep __no_const grsecurity-3.1-4.2.5-201511021814.patch | grep typedef | cut -d ' ' -f 3
>
> There are 32 of those.
>
> Then I compared to see if their structs were on your list.  For some
> reason there quite a few one their list which are not on yours.  Out
> of the first 10 about half weren't on your list.  cpu_cache_fns,
> outer_cache_fns, psci_operations, smp_operations, omap_hwmod_soc_ops,
> smp_ops_t.  These are mostly different arches?
>
> Also bit_table has in int has well as a function pointers but it is on
> their list.  I'm not sure why.  Maybe they are marking structs const
> that I don't know about.
>
> The other trick that they do is they define structs as __do_const if
> they want them to be const by default, which is pretty neat.  This feels
> like it should be a standard GCC feature.  In the meantime we could
> mark things as __do_const and print a sparse warning if it was declared
> as not const.
>
> I have attached the list of __no_const structs.

Thanks.  Architectures wouldn't matter for me, but an adjacent parsing
problem or a strangely written type name could cause a problem.  I will
check your list.

I would think though that the real problem wuld be things like the
platform_driver structure, which to my recollection has one non-constant
field, so the structure can't be const.

julia

  parent reply	other threads:[~2015-11-09  6:39 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-08 21:34 [PATCH] video: constify geode ops structures Julia Lawall
2015-11-08 21:34 ` Julia Lawall
2015-11-08 22:16 ` Dan Carpenter
2015-11-08 22:16   ` Dan Carpenter
2015-11-08 22:24   ` Julia Lawall
2015-11-08 22:24     ` Julia Lawall
2015-11-09  5:42     ` Dan Carpenter
2015-11-09  5:42       ` [kernel-hardening] " Dan Carpenter
2015-11-09  5:42       ` Dan Carpenter
2015-11-09  6:09       ` Joe Perches
2015-11-09  6:09         ` [kernel-hardening] " Joe Perches
2015-11-09  6:09         ` Joe Perches
2015-11-09  6:39       ` Julia Lawall [this message]
2015-11-09  6:39         ` [kernel-hardening] " Julia Lawall
2015-11-09  6:39         ` Julia Lawall
2015-11-09 13:30         ` [kernel-hardening] " Dan Carpenter
2015-11-09 13:30           ` Dan Carpenter
2015-11-09 18:12           ` Julia Lawall
2015-11-09 18:12             ` Julia Lawall
2015-11-09 18:19             ` Joe Perches
2015-11-09 18:19               ` Joe Perches
2015-11-09 13:49       ` Dan Carpenter
2015-11-09 13:49         ` Dan Carpenter
2015-11-09 14:50         ` Julia Lawall
2015-11-09 14:50           ` Julia Lawall
2015-11-09 16:39           ` Dan Carpenter
2015-11-09 16:39             ` Dan Carpenter
2015-11-09 17:05           ` Emese Revfy
2015-11-09 17:05             ` Emese Revfy
2015-11-09 17:48             ` Julia Lawall
2015-11-09 17:48               ` Julia Lawall
2015-11-09 21:24               ` Kees Cook
2015-11-09 21:24                 ` Kees Cook
2015-11-09 21:24                 ` Kees Cook
2015-11-09 21:55                 ` Julia Lawall
2015-11-09 21:55                   ` Julia Lawall
2015-11-09 21:55                   ` Julia Lawall
2015-11-09 23:34                   ` Kees Cook
2015-11-09 23:34                     ` Kees Cook
2015-11-09 23:34                     ` Kees Cook
2015-11-10  1:24                     ` PaX Team
2015-11-10  1:24                       ` PaX Team
2015-11-10  1:24                       ` PaX Team
2015-11-10 15:44       ` Julia Lawall
2015-11-10 15:44         ` [kernel-hardening] " Julia Lawall
2015-11-10 15:44         ` Julia Lawall
2015-11-09 21:20   ` Kees Cook
2015-11-09 21:20     ` Kees Cook
2015-11-10  6:38     ` Christoph Hellwig
2015-11-10  6:38       ` Christoph Hellwig
2015-11-10 20:34       ` Kees Cook
2015-11-10 20:34         ` Kees Cook
2015-11-10 20:49         ` Joe Perches
2015-11-10 20:49           ` Joe Perches
2015-11-10 22:02           ` Dan Carpenter
2015-11-10 22:02             ` Dan Carpenter
2015-11-10 22:17             ` Joe Perches
2015-11-10 22:17               ` Joe Perches
2015-11-10 22:34               ` Dan Carpenter
2015-11-10 22:34                 ` Dan Carpenter
2015-11-10 22:39                 ` Joe Perches
2015-11-10 22:39                   ` Joe Perches
2015-11-24 11:28 ` Tomi Valkeinen
2015-11-24 11:28   ` Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1511090634180.2593@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=dan.carpenter@oracle.com \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.