* status of constification
@ 2010-11-08 22:38 Kees Cook
2010-11-09 21:54 ` Emese Revfy
[not found] ` <4CD9BF25.4090306@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Kees Cook @ 2010-11-08 22:38 UTC (permalink / raw)
To: Emese Revfy, Lionel Debroux, linux-kernel
Hi,
So, I'm trying to come up to speed on what's still outstanding in the
effort to constify function pointers. I saw patches from Emese Revfy
go in, and I saw Lionel Debroux's recent patches. What already-created
work still needs attention? I know there's more code to be written,
but I'm trying to find any patches that have already been written but
have not gone upstream yet.
Thanks,
-Kees
--
Kees Cook
Ubuntu Security Team
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: status of constification 2010-11-08 22:38 status of constification Kees Cook @ 2010-11-09 21:54 ` Emese Revfy [not found] ` <4CD9BF25.4090306@gmail.com> 1 sibling, 0 replies; 14+ messages in thread From: Emese Revfy @ 2010-11-09 21:54 UTC (permalink / raw) To: Kees Cook; +Cc: Lionel Debroux, linux-kernel, Brad Spengler On 11/08/10 23:38, Kees Cook wrote: > Hi, > > So, I'm trying to come up to speed on what's still outstanding in the > effort to constify function pointers. I saw patches from Emese Revfy > go in, and I saw Lionel Debroux's recent patches. What already-created > work still needs attention? I know there's more code to be written, > but I'm trying to find any patches that have already been written but > have not gone upstream yet. Hi, I will gladly break up my current patch for the next -rc by structure type or maintainer (some preferred it one way or the other) and send it in some time next week so that you can handle the upstream submission process (I will continue to maintain my patch in grsecurity). There are many structures that can be constified, you can use the following command to find most of them (use it on an allyesconfig kernel preferably): grep _ops System.map |grep -Ewi 'b|d' | awk '{print $3}' | \ while read i ; do cscope -d -L -1 $i | grep -E "struct[ \t]*([^ ]*)[ \t]*" \ --color=none -o | awk '{print $2}' ; done |sort -u Also there are always new instances of structures going in that should have been constified. I tried to automate the whole process with Coccinelle but I abandoned it because Coccinelle didn't support recursive header file inclusion at the time. If someone feels like fixing Coccinelle then I would quickly finish my script (it has a few bugs because I could never test it for real), but see the end of the mail for the current version. I think it would be a good idea because it would take a few hours only to generate a constification patch for a new kernel. One thing that probably cannot be automated with Coccinelle is that once the script determines that a given structure cannot be constified, it cannot undo already emitted patches for the given structure so it must be cleaned up by post processing script. -- Emese // spatch.opt -sp_file $1 -include_headers -local_includes -all_includes -I "include/" -dir $2 @initialize:python@ noconst = [] @stc@ identifier idtype, y; type t; position p; @@ struct idtype { ... t (*y)(...);@p ... }; @notjustfp@ identifier stc.idtype, y; type t; position p != stc.p; @@ struct idtype { ... t y;@p ... }; @script:python depends on notjustfp@ @@ cocci.include_match(False) @variable@ identifier stc.idtype, idvariant, id; @@ ( struct idtype idvariant = { ... }; | struct idtype idvariant; | struct idtype *idvariant; | struct id { ... struct idtype idvariant; ... }; ) @script:python@ y << variable.idvariant; @@ if y in noconst: cocci.include_match(False) @alreadyconst@ identifier stc.idtype, variable.idvariant, id; @@ ( const struct idtype idvariant; | const struct idtype idvariant = { ... }; | const struct idtype *idvariant; | struct id { ... const struct idtype idvariant; ... }; ) @script:python depends on alreadyconst@ @@ cocci.include_match(False) @fn_declaration@ identifier stc.idtype, variable.idvariant, fn; type t; @@ t fn(struct idtype *idvariant); @fn_definition@ identifier stc.idtype, variable.idvariant, fn; type t; @@ t fn(struct idtype *idvariant) { ... } // TODO: handle var.field1.field2, var->field1->field2 @assignement@ identifier variable.idvariant, x, idptr; @@ ( idvariant.x = ...; | idvariant->x = ...; | idptr = &idvariant; ... idptr->x = ...; | memcpy(&idvariant, ...); | memcpy(idvariant.x, ...); | memcpy(idvariant->x, ...); | idvariant = kzalloc(...); | idvariant = kmalloc(...); ) @script:python depends on assignement@ x << stc.idtype; y << variable.idvariant; @@ print "Cannot be const: %s-%s" % (x, y) noconst.append(y) cocci.include_match(False) @depends on stc && !fn_declaration && !fn_definition@ identifier stc.idtype, variable.idvariant, id; @@ ( -struct idtype idvariant = { +const struct idtype idvariant = { ... }; | -struct idtype idvariant; +const struct idtype idvariant; | -struct idtype *idvariant; +const struct idtype *idvariant; | -struct idtype *idvariant = NULL; +const struct idtype *idvariant = NULL; | struct id { ... -struct idtype idvariant; +const struct idtype idvariant; ... }; ) @depends on stc && fn_declaration && !fn_definition@ identifier stc.idtype, variable.idvariant, fn_declaration.fn; type fn_declaration.t; @@ t fn( -struct idtype *idvariant +const struct idtype *idvariant ); @depends on stc && !fn_declaration && fn_definition@ identifier stc.idtype, variable.idvariant, fn_definition.fn; type fn_definition.t; @@ t fn( -struct idtype *idvariant +const struct idtype *idvariant ) { ... } ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <4CD9BF25.4090306@gmail.com>]
* Re: [Cocci] Re: status of constification [not found] ` <4CD9BF25.4090306@gmail.com> @ 2010-11-10 6:35 ` Julia Lawall 2010-11-11 21:48 ` Emese Revfy 2011-05-28 3:13 ` Kees Cook 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2010-11-10 6:35 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci On Tue, 9 Nov 2010, Emese Revfy wrote: > > > Hi, > > > > So, I'm trying to come up to speed on what's still outstanding in the > > effort to constify function pointers. I saw patches from Emese Revfy > > go in, and I saw Lionel Debroux's recent patches. What already-created > > work still needs attention? I know there's more code to be written, > > but I'm trying to find any patches that have already been written but > > have not gone upstream yet. > Hi, > > I will gladly break up my current patch for the next -rc by structure > type or maintainer (some preferred it one way or the other) and send > it in some time next week so that you can handle the upstream submission > process (I will continue to maintain my patch in grsecurity). > > There are many structures that can be constified, you can use the following > command to find most of them (use it on an allyesconfig kernel preferably): > > grep _ops System.map |grep -Ewi 'b|d' | awk '{print $3}' | \ > while read i ; do cscope -d -L -1 $i | grep -E "struct[ \t]*([^ ]*)[ \t]*" \ > --color=none -o | awk '{print $2}' ; done |sort -u > > Also there are always new instances of structures going in that should have > been constified. > > I tried to automate the whole process with Coccinelle but I abandoned it > because Coccinelle didn't support recursive header file inclusion at the time. > If someone feels like fixing Coccinelle then I would quickly finish my script > (it has a few bugs because I could never test it for real), but see the end > of the mail for the current version. I think it would be a good idea because > it would take a few hours only to generate a constification patch for a new > kernel. One thing that probably cannot be automated with Coccinelle is that > once the script determines that a given structure cannot be constified, it > cannot undo already emitted patches for the given structure so it must be > cleaned up by post processing script. What would the right approach be? It is not obvious to find 100% of the header files, because some of them depend on information in Makefiles. You can use that information by running the preprocessor on Coccinelle first, but then the result is only useful for finding files that need changing, but not actually making the changes because Coccinelle does not relate the preprocessed code back to the original code. But if you run the preprocessor, you only get information for your current configuration, which is probably not what you want. Coccinelle could certainly get a new option -really_all_includes, or something like that, that would recursively include among what it can find and what has not been included already. Would that be what is wanted? I guess that in practice the includes are only being used for type information? Wouldn't it be safe to run the semantic patch based on the includes that are available? julia > > -- > Emese > > > // spatch.opt -sp_file $1 -include_headers -local_includes -all_includes -I "include/" -dir $2 > > @initialize:python@ > noconst = [] > > @stc@ > identifier idtype, y; > type t; > position p; > @@ > struct idtype { > ... > t (*y)(...);@p > ... > }; > > @notjustfp@ > identifier stc.idtype, y; > type t; > position p != stc.p; > @@ > struct idtype { > ... > t y;@p > ... > }; > > @script:python depends on notjustfp@ > @@ > cocci.include_match(False) > > @variable@ > identifier stc.idtype, idvariant, id; > @@ > ( > struct idtype idvariant = { > ... > }; > | > struct idtype idvariant; > | > struct idtype *idvariant; > | > struct id { > ... > struct idtype idvariant; > ... > }; > ) > > @script:python@ > y << variable.idvariant; > @@ > if y in noconst: > cocci.include_match(False) > > @alreadyconst@ > identifier stc.idtype, variable.idvariant, id; > @@ > ( > const struct idtype idvariant; > | > const struct idtype idvariant = { > ... > }; > | > const struct idtype *idvariant; > | > struct id { > ... > const struct idtype idvariant; > ... > }; > ) > > @script:python depends on alreadyconst@ > @@ > cocci.include_match(False) > > @fn_declaration@ > identifier stc.idtype, variable.idvariant, fn; > type t; > @@ > t fn(struct idtype *idvariant); > > @fn_definition@ > identifier stc.idtype, variable.idvariant, fn; > type t; > @@ > t fn(struct idtype *idvariant) > { > ... > } > > // TODO: handle var.field1.field2, var->field1->field2 > @assignement@ > identifier variable.idvariant, x, idptr; > @@ > ( > idvariant.x = ...; > | > idvariant->x = ...; > | > idptr = &idvariant; > ... > idptr->x = ...; > | > memcpy(&idvariant, ...); > | > memcpy(idvariant.x, ...); > | > memcpy(idvariant->x, ...); > | > idvariant = kzalloc(...); > | > idvariant = kmalloc(...); > ) > > @script:python depends on assignement@ > x << stc.idtype; > y << variable.idvariant; > @@ > print "Cannot be const: %s-%s" % (x, y) > noconst.append(y) > cocci.include_match(False) > > @depends on stc && !fn_declaration && !fn_definition@ > identifier stc.idtype, variable.idvariant, id; > @@ > ( > -struct idtype idvariant = { > +const struct idtype idvariant = { > ... > }; > | > -struct idtype idvariant; > +const struct idtype idvariant; > | > -struct idtype *idvariant; > +const struct idtype *idvariant; > | > -struct idtype *idvariant = NULL; > +const struct idtype *idvariant = NULL; > | > struct id { > ... > -struct idtype idvariant; > +const struct idtype idvariant; > ... > }; > ) > > @depends on stc && fn_declaration && !fn_definition@ > identifier stc.idtype, variable.idvariant, fn_declaration.fn; > type fn_declaration.t; > @@ > t fn( > -struct idtype *idvariant > +const struct idtype *idvariant > ); > > @depends on stc && !fn_declaration && fn_definition@ > identifier stc.idtype, variable.idvariant, fn_definition.fn; > type fn_definition.t; > @@ > t fn( > -struct idtype *idvariant > +const struct idtype *idvariant > ) > { > ... > } > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2010-11-10 6:35 ` [Cocci] " Julia Lawall @ 2010-11-11 21:48 ` Emese Revfy 2010-11-11 22:53 ` Julia Lawall 2010-11-13 14:41 ` Julia Lawall 0 siblings, 2 replies; 14+ messages in thread From: Emese Revfy @ 2010-11-11 21:48 UTC (permalink / raw) To: Julia Lawall Cc: Kees Cook, linux-kernel, cocci, Lionel Debroux, Brad Spengler On 11/10/10 07:35, Julia Lawall wrote: >> I tried to automate the whole process with Coccinelle but I abandoned it >> because Coccinelle didn't support recursive header file inclusion at the time. >> If someone feels like fixing Coccinelle then I would quickly finish my script >> (it has a few bugs because I could never test it for real), but see the end >> of the mail for the current version. I think it would be a good idea because >> it would take a few hours only to generate a constification patch for a new >> kernel. One thing that probably cannot be automated with Coccinelle is that >> once the script determines that a given structure cannot be constified, it >> cannot undo already emitted patches for the given structure so it must be >> cleaned up by post processing script. > > What would the right approach be? It is not obvious to find 100% of the > header files, because some of them depend on information in Makefiles. For 100% coverage you can look at how the Linux Makefiles invoke sparse. > You can use that information by running the preprocessor on Coccinelle > first, but then the result is only useful for finding files that need > changing, but not actually making the changes because Coccinelle does not > relate the preprocessed code back to the original code. But if you run > the preprocessor, you only get information for your current configuration, > which is probably not what you want. > > Coccinelle could certainly get a new option -really_all_includes, or > something like that, that would recursively include among what it can find > and what has not been included already. Would that be what is wanted? Yes, this is exactly what I'd like to have, missing the few includes you referred to above is not a problem for my purposes. Thanks, Emese > > I guess that in practice the includes are only being used for type > information? Wouldn't it be safe to run the semantic patch based on the > includes that are available? > > julia > >> >> -- >> Emese >> >> >> // spatch.opt -sp_file $1 -include_headers -local_includes -all_includes -I "include/" -dir $2 >> >> @initialize:python@ >> noconst = [] >> >> @stc@ >> identifier idtype, y; >> type t; >> position p; >> @@ >> struct idtype { >> ... >> t (*y)(...);@p >> ... >> }; >> >> @notjustfp@ >> identifier stc.idtype, y; >> type t; >> position p != stc.p; >> @@ >> struct idtype { >> ... >> t y;@p >> ... >> }; >> >> @script:python depends on notjustfp@ >> @@ >> cocci.include_match(False) >> >> @variable@ >> identifier stc.idtype, idvariant, id; >> @@ >> ( >> struct idtype idvariant = { >> ... >> }; >> | >> struct idtype idvariant; >> | >> struct idtype *idvariant; >> | >> struct id { >> ... >> struct idtype idvariant; >> ... >> }; >> ) >> >> @script:python@ >> y << variable.idvariant; >> @@ >> if y in noconst: >> cocci.include_match(False) >> >> @alreadyconst@ >> identifier stc.idtype, variable.idvariant, id; >> @@ >> ( >> const struct idtype idvariant; >> | >> const struct idtype idvariant = { >> ... >> }; >> | >> const struct idtype *idvariant; >> | >> struct id { >> ... >> const struct idtype idvariant; >> ... >> }; >> ) >> >> @script:python depends on alreadyconst@ >> @@ >> cocci.include_match(False) >> >> @fn_declaration@ >> identifier stc.idtype, variable.idvariant, fn; >> type t; >> @@ >> t fn(struct idtype *idvariant); >> >> @fn_definition@ >> identifier stc.idtype, variable.idvariant, fn; >> type t; >> @@ >> t fn(struct idtype *idvariant) >> { >> ... >> } >> >> // TODO: handle var.field1.field2, var->field1->field2 >> @assignement@ >> identifier variable.idvariant, x, idptr; >> @@ >> ( >> idvariant.x = ...; >> | >> idvariant->x = ...; >> | >> idptr = &idvariant; >> ... >> idptr->x = ...; >> | >> memcpy(&idvariant, ...); >> | >> memcpy(idvariant.x, ...); >> | >> memcpy(idvariant->x, ...); >> | >> idvariant = kzalloc(...); >> | >> idvariant = kmalloc(...); >> ) >> >> @script:python depends on assignement@ >> x << stc.idtype; >> y << variable.idvariant; >> @@ >> print "Cannot be const: %s-%s" % (x, y) >> noconst.append(y) >> cocci.include_match(False) >> >> @depends on stc && !fn_declaration && !fn_definition@ >> identifier stc.idtype, variable.idvariant, id; >> @@ >> ( >> -struct idtype idvariant = { >> +const struct idtype idvariant = { >> ... >> }; >> | >> -struct idtype idvariant; >> +const struct idtype idvariant; >> | >> -struct idtype *idvariant; >> +const struct idtype *idvariant; >> | >> -struct idtype *idvariant = NULL; >> +const struct idtype *idvariant = NULL; >> | >> struct id { >> ... >> -struct idtype idvariant; >> +const struct idtype idvariant; >> ... >> }; >> ) >> >> @depends on stc && fn_declaration && !fn_definition@ >> identifier stc.idtype, variable.idvariant, fn_declaration.fn; >> type fn_declaration.t; >> @@ >> t fn( >> -struct idtype *idvariant >> +const struct idtype *idvariant >> ); >> >> @depends on stc && !fn_declaration && fn_definition@ >> identifier stc.idtype, variable.idvariant, fn_definition.fn; >> type fn_definition.t; >> @@ >> t fn( >> -struct idtype *idvariant >> +const struct idtype *idvariant >> ) >> { >> ... >> } >> >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2010-11-11 21:48 ` Emese Revfy @ 2010-11-11 22:53 ` Julia Lawall 2010-11-13 12:13 ` Emese Revfy 2010-11-13 14:41 ` Julia Lawall 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2010-11-11 22:53 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci, Lionel Debroux, Brad Spengler On Thu, 11 Nov 2010, Emese Revfy wrote: > On 11/10/10 07:35, Julia Lawall wrote: > >> I tried to automate the whole process with Coccinelle but I abandoned it > >> because Coccinelle didn't support recursive header file inclusion at the time. > >> If someone feels like fixing Coccinelle then I would quickly finish my script > >> (it has a few bugs because I could never test it for real), but see the end > >> of the mail for the current version. I think it would be a good idea because > >> it would take a few hours only to generate a constification patch for a new > >> kernel. One thing that probably cannot be automated with Coccinelle is that > >> once the script determines that a given structure cannot be constified, it > >> cannot undo already emitted patches for the given structure so it must be > >> cleaned up by post processing script. > > > > What would the right approach be? It is not obvious to find 100% of the > > header files, because some of them depend on information in Makefiles. > > For 100% coverage you can look at how the Linux Makefiles invoke sparse. I haven't looked at it, but I doubt it gives 100% coverage, because one can have code in both the if and else branches of an ifdef. I would imagine that it gives 100% coverage for whatever architecture you would be compiling for? julia > > You can use that information by running the preprocessor on Coccinelle > > first, but then the result is only useful for finding files that need > > changing, but not actually making the changes because Coccinelle does not > > relate the preprocessed code back to the original code. But if you run > > the preprocessor, you only get information for your current configuration, > > which is probably not what you want. > > > > Coccinelle could certainly get a new option -really_all_includes, or > > something like that, that would recursively include among what it can find > > and what has not been included already. Would that be what is wanted? > > Yes, this is exactly what I'd like to have, missing the few includes > you referred to above is not a problem for my purposes. > > Thanks, Emese > > > > > I guess that in practice the includes are only being used for type > > information? Wouldn't it be safe to run the semantic patch based on the > > includes that are available? > > > > julia > > > >> > >> -- > >> Emese > >> > >> > >> // spatch.opt -sp_file $1 -include_headers -local_includes -all_includes -I "include/" -dir $2 > >> > >> @initialize:python@ > >> noconst = [] > >> > >> @stc@ > >> identifier idtype, y; > >> type t; > >> position p; > >> @@ > >> struct idtype { > >> ... > >> t (*y)(...);@p > >> ... > >> }; > >> > >> @notjustfp@ > >> identifier stc.idtype, y; > >> type t; > >> position p != stc.p; > >> @@ > >> struct idtype { > >> ... > >> t y;@p > >> ... > >> }; > >> > >> @script:python depends on notjustfp@ > >> @@ > >> cocci.include_match(False) > >> > >> @variable@ > >> identifier stc.idtype, idvariant, id; > >> @@ > >> ( > >> struct idtype idvariant = { > >> ... > >> }; > >> | > >> struct idtype idvariant; > >> | > >> struct idtype *idvariant; > >> | > >> struct id { > >> ... > >> struct idtype idvariant; > >> ... > >> }; > >> ) > >> > >> @script:python@ > >> y << variable.idvariant; > >> @@ > >> if y in noconst: > >> cocci.include_match(False) > >> > >> @alreadyconst@ > >> identifier stc.idtype, variable.idvariant, id; > >> @@ > >> ( > >> const struct idtype idvariant; > >> | > >> const struct idtype idvariant = { > >> ... > >> }; > >> | > >> const struct idtype *idvariant; > >> | > >> struct id { > >> ... > >> const struct idtype idvariant; > >> ... > >> }; > >> ) > >> > >> @script:python depends on alreadyconst@ > >> @@ > >> cocci.include_match(False) > >> > >> @fn_declaration@ > >> identifier stc.idtype, variable.idvariant, fn; > >> type t; > >> @@ > >> t fn(struct idtype *idvariant); > >> > >> @fn_definition@ > >> identifier stc.idtype, variable.idvariant, fn; > >> type t; > >> @@ > >> t fn(struct idtype *idvariant) > >> { > >> ... > >> } > >> > >> // TODO: handle var.field1.field2, var->field1->field2 > >> @assignement@ > >> identifier variable.idvariant, x, idptr; > >> @@ > >> ( > >> idvariant.x = ...; > >> | > >> idvariant->x = ...; > >> | > >> idptr = &idvariant; > >> ... > >> idptr->x = ...; > >> | > >> memcpy(&idvariant, ...); > >> | > >> memcpy(idvariant.x, ...); > >> | > >> memcpy(idvariant->x, ...); > >> | > >> idvariant = kzalloc(...); > >> | > >> idvariant = kmalloc(...); > >> ) > >> > >> @script:python depends on assignement@ > >> x << stc.idtype; > >> y << variable.idvariant; > >> @@ > >> print "Cannot be const: %s-%s" % (x, y) > >> noconst.append(y) > >> cocci.include_match(False) > >> > >> @depends on stc && !fn_declaration && !fn_definition@ > >> identifier stc.idtype, variable.idvariant, id; > >> @@ > >> ( > >> -struct idtype idvariant = { > >> +const struct idtype idvariant = { > >> ... > >> }; > >> | > >> -struct idtype idvariant; > >> +const struct idtype idvariant; > >> | > >> -struct idtype *idvariant; > >> +const struct idtype *idvariant; > >> | > >> -struct idtype *idvariant = NULL; > >> +const struct idtype *idvariant = NULL; > >> | > >> struct id { > >> ... > >> -struct idtype idvariant; > >> +const struct idtype idvariant; > >> ... > >> }; > >> ) > >> > >> @depends on stc && fn_declaration && !fn_definition@ > >> identifier stc.idtype, variable.idvariant, fn_declaration.fn; > >> type fn_declaration.t; > >> @@ > >> t fn( > >> -struct idtype *idvariant > >> +const struct idtype *idvariant > >> ); > >> > >> @depends on stc && !fn_declaration && fn_definition@ > >> identifier stc.idtype, variable.idvariant, fn_definition.fn; > >> type fn_definition.t; > >> @@ > >> t fn( > >> -struct idtype *idvariant > >> +const struct idtype *idvariant > >> ) > >> { > >> ... > >> } > >> > >> > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2010-11-11 22:53 ` Julia Lawall @ 2010-11-13 12:13 ` Emese Revfy 2010-11-13 13:43 ` Julia Lawall 0 siblings, 1 reply; 14+ messages in thread From: Emese Revfy @ 2010-11-13 12:13 UTC (permalink / raw) To: Julia Lawall Cc: Kees Cook, linux-kernel, cocci, Lionel Debroux, Brad Spengler >>> What would the right approach be? It is not obvious to find 100% of the >>> header files, because some of them depend on information in Makefiles. >> >> For 100% coverage you can look at how the Linux Makefiles invoke sparse. > > I haven't looked at it, but I doubt it gives 100% coverage, because one > can have code in both the if and else branches of an ifdef. I would > imagine that it gives 100% coverage for whatever architecture you would be > compiling for? For my purposes it's enough to find all includes for a given .config. What would be also useful is if Coccinelle could somehow ignore ifdef's that guard code blocks so that it would analyse all code in a translation unit. Thanks Emese ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2010-11-13 12:13 ` Emese Revfy @ 2010-11-13 13:43 ` Julia Lawall 0 siblings, 0 replies; 14+ messages in thread From: Julia Lawall @ 2010-11-13 13:43 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci, Lionel Debroux, Brad Spengler On Sat, 13 Nov 2010, Emese Revfy wrote: > >>> What would the right approach be? It is not obvious to find 100% of the > >>> header files, because some of them depend on information in Makefiles. > >> > >> For 100% coverage you can look at how the Linux Makefiles invoke sparse. > > > > I haven't looked at it, but I doubt it gives 100% coverage, because one > > can have code in both the if and else branches of an ifdef. I would > > imagine that it gives 100% coverage for whatever architecture you would be > > compiling for? > > For my purposes it's enough to find all includes for a given .config. Coccinelle doesn't interpret .config files. > What would be also useful is if Coccinelle could somehow ignore ifdef's that > guard code blocks so that it would analyse all code in a translation unit. It does that. julia ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2010-11-11 21:48 ` Emese Revfy 2010-11-11 22:53 ` Julia Lawall @ 2010-11-13 14:41 ` Julia Lawall 1 sibling, 0 replies; 14+ messages in thread From: Julia Lawall @ 2010-11-13 14:41 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci, Lionel Debroux, Brad Spengler Recursive includes will be available shortly, with the option -recursive_includes. julia ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: status of constification [not found] ` <4CD9BF25.4090306@gmail.com> 2010-11-10 6:35 ` [Cocci] " Julia Lawall @ 2011-05-28 3:13 ` Kees Cook 2011-05-28 6:39 ` [Cocci] " Julia Lawall 1 sibling, 1 reply; 14+ messages in thread From: Kees Cook @ 2011-05-28 3:13 UTC (permalink / raw) To: Emese Revfy; +Cc: Lionel Debroux, linux-kernel, cocci Hi Emese, I got distracted, but I'd like to get back to this thread... On Tue, Nov 09, 2010 at 10:37:41PM +0100, Emese Revfy wrote: > I will gladly break up my current patch for the next -rc by structure > type or maintainer (some preferred it one way or the other) and send > it in some time next week so that you can handle the upstream submission > process (I will continue to maintain my patch in grsecurity). > > There are many structures that can be constified, you can use the following > command to find most of them (use it on an allyesconfig kernel preferably): > > grep _ops System.map |grep -Ewi 'b|d' | awk '{print $3}' | \ > while read i ; do cscope -d -L -1 $i | grep -E "struct[ \t]*([^ ]*)[ \t]*" \ > --color=none -o | awk '{print $2}' ; done |sort -u > > Also there are always new instances of structures going in that should have > been constified. Just in my running kernel, I see 56 _ops structures reported from the above search. :) Do you have a new stack of patches I can help usher into the kernel? I don't want reinvent the wheel if I don't have to. :) > I tried to automate the whole process with Coccinelle but I abandoned it > because Coccinelle didn't support recursive header file inclusion at the time. > If someone feels like fixing Coccinelle then I would quickly finish my script > (it has a few bugs because I could never test it for real), but see the end > of the mail for the current version. I think it would be a good idea because > it would take a few hours only to generate a constification patch for a new > kernel. One thing that probably cannot be automated with Coccinelle is that > once the script determines that a given structure cannot be constified, it > cannot undo already emitted patches for the given structure so it must be > cleaned up by post processing script. Has there been any update to your Coccinelle script since the addition of -recursive_includes? Thanks! -Kees -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2011-05-28 3:13 ` Kees Cook @ 2011-05-28 6:39 ` Julia Lawall 2011-06-05 21:47 ` Emese Revfy 0 siblings, 1 reply; 14+ messages in thread From: Julia Lawall @ 2011-05-28 6:39 UTC (permalink / raw) To: Kees Cook; +Cc: Emese Revfy, linux-kernel, cocci On Fri, 27 May 2011, Kees Cook wrote: > Hi Emese, > > I got distracted, but I'd like to get back to this thread... > > On Tue, Nov 09, 2010 at 10:37:41PM +0100, Emese Revfy wrote: > > I will gladly break up my current patch for the next -rc by structure > > type or maintainer (some preferred it one way or the other) and send > > it in some time next week so that you can handle the upstream submission > > process (I will continue to maintain my patch in grsecurity). > > > > There are many structures that can be constified, you can use the following > > command to find most of them (use it on an allyesconfig kernel preferably): > > > > grep _ops System.map |grep -Ewi 'b|d' | awk '{print $3}' | \ > > while read i ; do cscope -d -L -1 $i | grep -E "struct[ \t]*([^ ]*)[ \t]*" \ > > --color=none -o | awk '{print $2}' ; done |sort -u > > > > Also there are always new instances of structures going in that should have > > been constified. > > Just in my running kernel, I see 56 _ops structures reported from the above > search. :) > > Do you have a new stack of patches I can help usher into the kernel? I > don't want reinvent the wheel if I don't have to. :) > > > I tried to automate the whole process with Coccinelle but I abandoned it > > because Coccinelle didn't support recursive header file inclusion at the time. > > If someone feels like fixing Coccinelle then I would quickly finish my script > > (it has a few bugs because I could never test it for real), but see the end > > of the mail for the current version. I think it would be a good idea because > > it would take a few hours only to generate a constification patch for a new > > kernel. One thing that probably cannot be automated with Coccinelle is that > > once the script determines that a given structure cannot be constified, it > > cannot undo already emitted patches for the given structure so it must be > > cleaned up by post processing script. Could I see the semantic patch? The clean up issue sounds interesting. Perhaps there is a way around it. julia > Has there been any update to your Coccinelle script since the addition of > -recursive_includes? > > Thanks! > > -Kees > > -- > Kees Cook > Ubuntu Security Team > _______________________________________________ > Cocci mailing list > Cocci@diku.dk > http://lists.diku.dk/mailman/listinfo/cocci > (Web access from inside DIKUs LAN only) > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2011-05-28 6:39 ` [Cocci] " Julia Lawall @ 2011-06-05 21:47 ` Emese Revfy 2011-06-06 5:03 ` Julia Lawall 2011-06-06 7:49 ` Julia Lawall 0 siblings, 2 replies; 14+ messages in thread From: Emese Revfy @ 2011-06-05 21:47 UTC (permalink / raw) To: Julia Lawall; +Cc: Kees Cook, linux-kernel, cocci > Could I see the semantic patch? The clean up issue sounds interesting. > Perhaps there is a way around it. Hi, I no longer use Coccinelle to generate constification patch, but only to find candidate structures for constification. This is because I ran into several issues some of which I will try describe now from memory (unfortunately I don't have those Cocci scripts anymore): 1. Sometimes (always?) cocci removed NULL initialization while emitting the constified patch: linux-2.6.39-rc7/arch/frv/mb93090-mb00/pci-vdk.c @@ -356,7 +356,7 @@ void __init pcibios_fixup_bus(struct pci int __init pcibios_init(void) { - struct pci_ops *dir = NULL; + const struct pci_ops *dir; if (!mb93090_mb00_detected) return -ENXIO; 2. Sometimes (always?) cocci joined structure field lines into one line while constifying them: linux-2.6.39-rc7/arch/x86/include/asm/x86_init.h @@ -119,14 +119,7 @@ struct x86_init_pci { * */ struct x86_init_ops { - struct x86_init_resources resources; - struct x86_init_mpparse mpparse; - struct x86_init_irqs irqs; - struct x86_init_oem oem; - struct x86_init_paging paging; - struct x86_init_timers timers; - struct x86_init_iommu iommu; - struct x86_init_pci pci; + const struct x86_init_resources resources;const struct x86_init_mpparse mpparse;const struct x86_init_irqs irqs;const struct x86_init_oem oem;const struct x86_init_paging paging;const struct x86_init_timers timers;const struct x86_init_iommu iommu;const struct x86_init_pci pci; }; /** 3. Sometimes cocci removed comment lines while joining others, maybe related to the previous issue: linux-2.6.39-rc7/drivers/net/wireless/ath/ath9k/hw.h @@ -753,9 +753,7 @@ struct ath_hw { } enable_32kHz_clock; /* Private to hardware code */ - struct ath_hw_private_ops private_ops; - /* Accessed by the lower level driver */ - struct ath_hw_ops ops; + const struct ath_hw_private_ops private_ops;const struct ath_hw_ops ops; /* Used to program the radio on non single-chip devices */ u32 *analogBank0Data; 4. My constification patch generator cocci script didn't find all structures that it should have, I don't know whether it was a bug in my script or in cocci but in the end I just gave it up. -- Emese ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2011-06-05 21:47 ` Emese Revfy @ 2011-06-06 5:03 ` Julia Lawall 2011-06-06 7:49 ` Julia Lawall 1 sibling, 0 replies; 14+ messages in thread From: Julia Lawall @ 2011-06-06 5:03 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci Thanks for the reports. The first is due to an isomorphism that allows a declaration to match a declaration in which the declared variable is assigned to a constant. You can put disable const_decl_init in your rule to avoid this. I'll look into the disappearing newlines problem. I think there has been some progress in this direction recently, but proper pretty printing is an ongoing battle... I'm not sure about the last problem. If you try again at some point, you can let me know in more detail of what the problem is. Thanks, julia On Sun, 5 Jun 2011, Emese Revfy wrote: > > Could I see the semantic patch? The clean up issue sounds interesting. > > Perhaps there is a way around it. > > Hi, > I no longer use Coccinelle to generate constification patch, but only to find > candidate structures for constification. This is because I ran into several > issues > some of which I will try describe now from memory (unfortunately I don't have > those Cocci scripts anymore): > > 1. Sometimes (always?) cocci removed NULL initialization while emitting the > constified patch: > > linux-2.6.39-rc7/arch/frv/mb93090-mb00/pci-vdk.c > @@ -356,7 +356,7 @@ void __init pcibios_fixup_bus(struct pci > > int __init pcibios_init(void) > { > - struct pci_ops *dir = NULL; > + const struct pci_ops *dir; > > if (!mb93090_mb00_detected) > return -ENXIO; > > 2. Sometimes (always?) cocci joined structure field lines into one line while > constifying them: > > linux-2.6.39-rc7/arch/x86/include/asm/x86_init.h > @@ -119,14 +119,7 @@ struct x86_init_pci { > * > */ > struct x86_init_ops { > - struct x86_init_resources resources; > - struct x86_init_mpparse mpparse; > - struct x86_init_irqs irqs; > - struct x86_init_oem oem; > - struct x86_init_paging paging; > - struct x86_init_timers timers; > - struct x86_init_iommu iommu; > - struct x86_init_pci pci; > + const struct x86_init_resources resources;const struct > x86_init_mpparse mpparse;const struct x86_init_irqs irqs;const struct > x86_init_oem oem;const struct x86_init_paging paging;const struct > x86_init_timers timers;const struct x86_init_iommu iommu;const struct > x86_init_pci pci; > }; > > /** > > 3. Sometimes cocci removed comment lines while joining others, maybe related > to the previous issue: > linux-2.6.39-rc7/drivers/net/wireless/ath/ath9k/hw.h > @@ -753,9 +753,7 @@ struct ath_hw { > } enable_32kHz_clock; > > /* Private to hardware code */ > - struct ath_hw_private_ops private_ops; > - /* Accessed by the lower level driver */ > - struct ath_hw_ops ops; > + const struct ath_hw_private_ops private_ops;const struct ath_hw_ops > ops; > > /* Used to program the radio on non single-chip devices */ > u32 *analogBank0Data; > > 4. My constification patch generator cocci script didn't find all structures > that it should have, I don't know whether it was a bug in my script or in > cocci but in the end I just gave it up. > > -- > Emese > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2011-06-05 21:47 ` Emese Revfy 2011-06-06 5:03 ` Julia Lawall @ 2011-06-06 7:49 ` Julia Lawall 2011-06-10 21:28 ` Emese Revfy 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2011-06-06 7:49 UTC (permalink / raw) To: Emese Revfy; +Cc: Kees Cook, linux-kernel, cocci On Sun, 5 Jun 2011, Emese Revfy wrote: > > Could I see the semantic patch? The clean up issue sounds interesting. > > Perhaps there is a way around it. > > Hi, > I no longer use Coccinelle to generate constification patch, but only to find > candidate structures for constification. This is because I ran into several > issues > some of which I will try describe now from memory (unfortunately I don't have > those Cocci scripts anymore): > > 1. Sometimes (always?) cocci removed NULL initialization while emitting the > constified patch: > > linux-2.6.39-rc7/arch/frv/mb93090-mb00/pci-vdk.c > @@ -356,7 +356,7 @@ void __init pcibios_fixup_bus(struct pci > > int __init pcibios_init(void) > { > - struct pci_ops *dir = NULL; > + const struct pci_ops *dir; > > if (!mb93090_mb00_detected) > return -ENXIO; > > 2. Sometimes (always?) cocci joined structure field lines into one line while > constifying them: > > linux-2.6.39-rc7/arch/x86/include/asm/x86_init.h > @@ -119,14 +119,7 @@ struct x86_init_pci { > * > */ > struct x86_init_ops { > - struct x86_init_resources resources; > - struct x86_init_mpparse mpparse; > - struct x86_init_irqs irqs; > - struct x86_init_oem oem; > - struct x86_init_paging paging; > - struct x86_init_timers timers; > - struct x86_init_iommu iommu; > - struct x86_init_pci pci; > + const struct x86_init_resources resources;const struct > x86_init_mpparse mpparse;const struct x86_init_irqs irqs;const struct > x86_init_oem oem;const struct x86_init_paging paging;const struct > x86_init_timers timers;const struct x86_init_iommu iommu;const struct > x86_init_pci pci; > }; This problem seem to be solved. At least, I don't see the problem for the following semantic patch: @@ identifier i,x; type T; @@ struct i { ... - T x; + const T x; ... }; on the following code: struct i { int a; int b; /* a comment */ int c; int d; }; The comment is preserved as well. julia > /** > > 3. Sometimes cocci removed comment lines while joining others, maybe related > to the previous issue: > linux-2.6.39-rc7/drivers/net/wireless/ath/ath9k/hw.h > @@ -753,9 +753,7 @@ struct ath_hw { > } enable_32kHz_clock; > > /* Private to hardware code */ > - struct ath_hw_private_ops private_ops; > - /* Accessed by the lower level driver */ > - struct ath_hw_ops ops; > + const struct ath_hw_private_ops private_ops;const struct ath_hw_ops > ops; > > /* Used to program the radio on non single-chip devices */ > u32 *analogBank0Data; > > 4. My constification patch generator cocci script didn't find all structures > that it should have, I don't know whether it was a bug in my script or in > cocci but in the end I just gave it up. > > -- > Emese > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Cocci] Re: status of constification 2011-06-06 7:49 ` Julia Lawall @ 2011-06-10 21:28 ` Emese Revfy 0 siblings, 0 replies; 14+ messages in thread From: Emese Revfy @ 2011-06-10 21:28 UTC (permalink / raw) To: Julia Lawall; +Cc: Kees Cook, linux-kernel, cocci > This problem seem to be solved. At least, I don't see the problem for the > following semantic patch: > > @@ > identifier i,x; > type T; > @@ > > struct i { > ... > - T x; > + const T x; > ... > }; > > on the following code: > > struct i { > int a; > int b; > /* a comment */ > int c; > int d; > }; > > The comment is preserved as well. Hi, I've just noticed there is a new version out (1.0.0-rc3), I will give it a try and let you know the results. -- Emese ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-06-10 21:27 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-08 22:38 status of constification Kees Cook
2010-11-09 21:54 ` Emese Revfy
[not found] ` <4CD9BF25.4090306@gmail.com>
2010-11-10 6:35 ` [Cocci] " Julia Lawall
2010-11-11 21:48 ` Emese Revfy
2010-11-11 22:53 ` Julia Lawall
2010-11-13 12:13 ` Emese Revfy
2010-11-13 13:43 ` Julia Lawall
2010-11-13 14:41 ` Julia Lawall
2011-05-28 3:13 ` Kees Cook
2011-05-28 6:39 ` [Cocci] " Julia Lawall
2011-06-05 21:47 ` Emese Revfy
2011-06-06 5:03 ` Julia Lawall
2011-06-06 7:49 ` Julia Lawall
2011-06-10 21:28 ` Emese Revfy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox