Re: matching an arbitrary struct or union (but not scalars)

Re: matching an arbitrary struct or union (but not scalars)

[Mansour Moufid]

On Thu, Sep 16, 2021 at 7:48 PM Kees Cook <keescook@chromium.org> wrote:
>
> Hi,
>
> I'd like to match any compound type containing a compound type, i.e. these:
>
> struct foo {
>         struct bar instance;
> };
>
> union bar {
>         int c;
>         struct baz d;
> };

For this first part, maybe something like:

@@
identifier s, u;
type t1 = {struct s, union u};
type t2 = {struct s, union u};
identifier x;
@@
*   t1 {
        ...
*       t2 x;
        ...
    }

I'll attempt the rest tomorrow.

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

On Thu, 16 Sep 2021, Kees Cook wrote:

> Hi,
>
> I'd like to match any compound type containing a compound type, i.e. these:
>
> struct foo {
> 	struct bar instance;
> };
>
> union bar {
> 	int c;
> 	struct baz d;
> };
>
> char buf[5][9];
>
> and the uuid_t type, where a zero initializer is used:
>
> struct foo instance = { 0 };
>
> Even in the simple case, I would expect this to be something like:
>
> @zero_initializer@
> ???? T;
> identifier I;
>
>  T I = {
> -	 0
>  };
>
> But I do not want to match scalars and single-dimensional scalar arrays
> like these:
>
> 	char name[8] = { 0 };
>
> so I can't just use "type T".
>
>
> I tried:
>
> @@
> type T =~ "^((struct|union) .*|uuid_t)$";
> identifier I;
> @@
>
>  T I = {
> - 0
>  };
>
> but this only matched uuid_t instances, which I don't understand.

@@
identifier i,x;
typedef uuid_t;
@@

(
struct i
|
union i
|
uuid_t
)
x = {
- 0
  };

> For a more exact match, this works for finding a struct within a struct:
>
> @found_compound@
> identifier COMPOUND, SUB, NAME;
> @@
>
>  struct COMPOUND {
>         ...
>         struct SUB NAME;
>         ...
>  };
>
> @zero_init depends on found_struct@
> identifier found_compound.COMPOUND;
> identifier I;
> @@
>
>  struct COMPOUND I = {
> - 0
>  };
>
>
> But this doesn't (fails to compile and I don't know why):
>
> @found_compound@
> identifier COMPOUND, SUB, NAME;
> @@
>
>  \(struct\|union\) COMPOUND {
>         ...
>         \(struct\|union\) SUB NAME;
>         ...
>  };
>
> @zero_init depends on found_struct@
> identifier found_compound.COMPOUND;
> identifier I;
> @@
>
>  \(struct\|union\) COMPOUND I = {
> - 0
>  };

Disjunctions are only allowed around complete C terms.  In struct x, the
type is struct x, not just struct.  So you need to duplicate COMPOUND in
each branch.

In the found_compound case it seems that one has to duplicate the entire
type declaration:

@found_compound@
identifier COMPOUND, SUB, NAME;
@@

(
  struct COMPOUND {
         ...
         \(struct SUB \|union SUB\) NAME;
         ...
  };
|
  union COMPOUND {
         ...
         \(struct SUB \|union SUB\) NAME;
         ...
  };
)

julia

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

On Fri, 17 Sep 2021, Mansour Moufid wrote:

> On Thu, Sep 16, 2021 at 7:48 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Hi,
> >
> > I'd like to match any compound type containing a compound type, i.e. these:
> >
> > struct foo {
> >         struct bar instance;
> > };
> >
> > union bar {
> >         int c;
> >         struct baz d;
> > };
>
> For this first part, maybe something like:
>
> @@
> identifier s, u;
> type t1 = {struct s, union u};
> type t2 = {struct s, union u};
> identifier x;
> @@
> *   t1 {
>         ...
> *       t2 x;
>         ...
>     }

I think that this should also be ok.

julia

>
> I'll attempt the rest tomorrow.
>

Re: matching an arbitrary struct or union (but not scalars)

[Mansour Moufid]

On Thu, Sep 16, 2021 at 7:48 PM Kees Cook <keescook@chromium.org> wrote:
>
> Hi,
>
> I'd like to match any compound type containing a compound type, i.e. these:
>
> struct foo {
>         struct bar instance;
> };
>
> union bar {
>         int c;
>         struct baz d;
> };
>
> char buf[5][9];
>
> and the uuid_t type, where a zero initializer is used:
>
> struct foo instance = { 0 };
>
> [...]
>
> But I do not want to match scalars and single-dimensional scalar arrays
> like these:
>
>         char name[8] = { 0 };

This:

@@
type t;
identifier x;
@@
*   t x[...][...];

will match two or more dimensional arrays, like x[1][2] and x[][2] as
well as x[1][2][3] and so on.

So altogether a script could look like this:

@a@
identifier s, u;
type t1 = {struct s, union u};
type t2 = {struct s, union u};
identifier x;
@@
    t1 {
        ...
        t2 x;
        ...
    }

@@
typedef uuid_t;
type t = {a.t1, uuid_t};
identifier x;
@@
    t x[...] = {
-       0
    };

@@
type t;
identifier x;
@@
    t x[...][...] = {
-       0
    };

I assume this is for the Linux kernel? It looks like there are plenty
of variables of those types but none initialized to {0}. (Although the
script is not done running through the entire kernel, that takes
forever.)

Re: matching an arbitrary struct or union (but not scalars)

[Kees Cook]

On Fri, Sep 17, 2021 at 03:16:21PM -0400, Mansour Moufid wrote:
> This:
> 
> @@
> type t;
> identifier x;
> @@
> *   t x[...][...];
> 
> will match two or more dimensional arrays, like x[1][2] and x[][2] as
> well as x[1][2][3] and so on.
> 
> So altogether a script could look like this:
> 
> @a@
> identifier s, u;
> type t1 = {struct s, union u};
> type t2 = {struct s, union u};
> identifier x;
> @@
>     t1 {
>         ...
>         t2 x;
>         ...
>     }
> 
> @@
> typedef uuid_t;
> type t = {a.t1, uuid_t};
> identifier x;
> @@
>     t x[...] = {
> -       0
>     };
> 
> @@
> type t;
> identifier x;
> @@
>     t x[...][...] = {
> -       0
>     };
> 
> I assume this is for the Linux kernel? It looks like there are plenty
> of variables of those types but none initialized to {0}. (Although the
> script is not done running through the entire kernel, that takes
> forever.)

Yeah, this is for the kernel. Thanks for helping with this! I was able
to continue the construction and get it working. :)
Here's a test case:


#include <stdio.h>
#include <uuid/uuid.h>

/* Ignore 0-init (no internal compound type) */
struct only_scalars_struct {
	int a;
	short b;
	unsigned long c;
	void *ptr;
};

/* has internal struct */
struct has_struct {
	int a;
	struct only_scalars_struct inner;
	void *ptr;
};

/* has internal array */
struct has_array {
	int a;
	char buf[8];
	void *ptr;
};

/* has internal compound type (uuid_t) */
struct has_uuid {
	int a;
	uuid_t id;
	void *ptr;
};

/* has internal array of struct */
struct has_struct_array {
	int a;
	struct only_scalars_struct many[4];
	void *ptr;
};

/* internally defined struct */
struct has_internal_struct_def {
	int a;
	struct foo {
		int b;
		int c;
	} inside;
	void *ptr;
};

/* Ignore 0-init: no internal compound types */
union only_scalars_union {
	int a;
	short b;
	void *ptr;
};

/* has internal union */
struct has_union {
	union only_scalars_union z;
	void *ptr;
};

/* internally defined union */
union has_internal_union_def {
	union {
		int a;
		long b;
	} z;
	void *ptr;
};

/* has internal compound types */
union of_many {
	struct only_scalars_struct inside;
	struct has_union stuffed;
	union only_scalars_union scalars;
};

/* has internal array */
union of_array {
	char buf[4];
	void *ptr;
};

/* has internal compound type */
union of_uuid {
	void *ptr;
	uuid_t id;
};

int main(void)
{
	struct only_scalars_struct	a = { 0 }; // should be ignored
	struct has_struct		b = { 0 };
	struct has_union		c = { 0 };
	struct has_uuid			d = { 0 };
	struct has_array		e = { 0 };
	struct has_struct_array		f = { 0 };
	struct has_internal_struct_def	g = { 0 };

	union only_scalars_union	h = { 0 }; // should be ignored
	union of_many			i = { 0 };
	union of_array			j = { 0 };
	union of_uuid			k = { 0 };
	union has_internal_union_def	l = { 0 };

	char one[16] = { 0 };	// should be ignored
	char two[16][5] = { 0 };
	uuid_t uuid = { 0 };
	struct only_scalars_struct structs[4] = { 0 };
	union only_scalars_union unions[4] = { 0 };

	puts("hello");

	return 0;
}


And here's the cocci:


@compound@
typedef uuid_t;
identifier os, is, ou, iu;
type outer = {struct os, union ou};
type inner = {struct is, union iu, uuid_t};
type t;
identifier x;
@@

    outer {
        ...
(
        inner x;
|
        inner x[...];
|
        t x[...];
|
	inner {
		...
	} x;
|
	inner {
		...
	} x[...];
)
        ...
    };

@single@
typedef uuid_t;
type t = {compound.outer, uuid_t};
identifier x;
@@

    t x = {
-       0
    };

@array_of_compound_type@
typedef uuid_t;
identifier s, u;
type t = {struct s, union u, uuid_t};
identifier x;
@@
    t x[...] = {
-       0
    };

@multi_dimensional_array_of_anything@
type t;
identifier x;
@@
    t x[...][...] = {
-       0
    };



But it emits a bunch of warnings:

warning: compound: metavariable os not used in the - or context code
warning: compound: metavariable is not used in the - or context code
warning: compound: metavariable iu not used in the - or context code
warning: compound: metavariable ou not used in the - or context code
warning: array_of_compound_type: metavariable u not used in the - or context code
warning: array_of_compound_type: metavariable s not used in the - or context code

Can these be silenced in some sane way?

Thanks!

-Kees

-- 
Kees Cook

Re: matching an arbitrary struct or union (but not scalars)

[Kees Cook]

On Sat, Sep 18, 2021 at 08:40:19AM -0700, Kees Cook wrote:
> Yeah, this is for the kernel. Thanks for helping with this! I was able
> to continue the construction and get it working. :)

I spoke too soon; it fails on the kernel with:
EXN: Coccinelle_modules.Common.Timeout

any ideas?

-- 
Kees Cook

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

> But it emits a bunch of warnings:
>
> warning: compound: metavariable os not used in the - or context code
> warning: compound: metavariable is not used in the - or context code
> warning: compound: metavariable iu not used in the - or context code
> warning: compound: metavariable ou not used in the - or context code
> warning: array_of_compound_type: metavariable u not used in the - or context code
> warning: array_of_compound_type: metavariable s not used in the - or context code
>
> Can these be silenced in some sane way?

Surely.  I'll take a look.

julia

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

On Sat, 18 Sep 2021, Kees Cook wrote:

> On Sat, Sep 18, 2021 at 08:40:19AM -0700, Kees Cook wrote:
> > Yeah, this is for the kernel. Thanks for helping with this! I was able
> > to continue the construction and get it working. :)
>
> I spoke too soon; it fails on the kernel with:
> EXN: Coccinelle_modules.Common.Timeout

What is your command line?  If it times out on one file, it should just
move on to the next one.

julia

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

> But it emits a bunch of warnings:
>
> warning: compound: metavariable os not used in the - or context code
> warning: compound: metavariable is not used in the - or context code
> warning: compound: metavariable iu not used in the - or context code
> warning: compound: metavariable ou not used in the - or context code
> warning: array_of_compound_type: metavariable u not used in the - or context code
> warning: array_of_compound_type: metavariable s not used in the - or context code
>
> Can these be silenced in some sane way?

The problem is fixed.

julia

Re: matching an arbitrary struct or union (but not scalars)

[Kees Cook]

On Sat, Sep 18, 2021 at 08:50:57PM +0200, Julia Lawall wrote:
> > But it emits a bunch of warnings:
> >
> > warning: compound: metavariable os not used in the - or context code
> > warning: compound: metavariable is not used in the - or context code
> > warning: compound: metavariable iu not used in the - or context code
> > warning: compound: metavariable ou not used in the - or context code
> > warning: array_of_compound_type: metavariable u not used in the - or context code
> > warning: array_of_compound_type: metavariable s not used in the - or context code
> >
> > Can these be silenced in some sane way?
> 
> The problem is fixed.

Awesome; thank you! I'll need to start building coccinelle from git now.
:)

-- 
Kees Cook

Re: matching an arbitrary struct or union (but not scalars)

[Kees Cook]

On Sat, Sep 18, 2021 at 06:13:32PM +0200, Julia Lawall wrote:
> 
> 
> On Sat, 18 Sep 2021, Kees Cook wrote:
> 
> > On Sat, Sep 18, 2021 at 08:40:19AM -0700, Kees Cook wrote:
> > > Yeah, this is for the kernel. Thanks for helping with this! I was able
> > > to continue the construction and get it working. :)
> >
> > I spoke too soon; it fails on the kernel with:
> > EXN: Coccinelle_modules.Common.Timeout
> 
> What is your command line?  If it times out on one file, it should just
> move on to the next one.

To work around potential intermixed output, I rewrite the normal
coccicheck arguments that uses --jobs to launch $nproc many spatch
instances with -max and -index. I will switch back to using --jobs and
see if I still get corrupted patches... (It's been a while since I
created this alternative workflow.)

-Kees

-- 
Kees Cook

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

On Mon, 20 Sep 2021, Kees Cook wrote:

> On Sat, Sep 18, 2021 at 08:50:57PM +0200, Julia Lawall wrote:
> > > But it emits a bunch of warnings:
> > >
> > > warning: compound: metavariable os not used in the - or context code
> > > warning: compound: metavariable is not used in the - or context code
> > > warning: compound: metavariable iu not used in the - or context code
> > > warning: compound: metavariable ou not used in the - or context code
> > > warning: array_of_compound_type: metavariable u not used in the - or context code
> > > warning: array_of_compound_type: metavariable s not used in the - or context code
> > >
> > > Can these be silenced in some sane way?
> >
> > The problem is fixed.
>
> Awesome; thank you! I'll need to start building coccinelle from git now.
> :)

Yes, we are not so proactive about making releases.  One should come soon,
but there is a build-related problem that needs to be addressed in the
meantime.

julia

Re: matching an arbitrary struct or union (but not scalars)

[Julia Lawall]

On Mon, 20 Sep 2021, Kees Cook wrote:

> On Sat, Sep 18, 2021 at 06:13:32PM +0200, Julia Lawall wrote:
> >
> >
> > On Sat, 18 Sep 2021, Kees Cook wrote:
> >
> > > On Sat, Sep 18, 2021 at 08:40:19AM -0700, Kees Cook wrote:
> > > > Yeah, this is for the kernel. Thanks for helping with this! I was able
> > > > to continue the construction and get it working. :)
> > >
> > > I spoke too soon; it fails on the kernel with:
> > > EXN: Coccinelle_modules.Common.Timeout
> >
> > What is your command line?  If it times out on one file, it should just
> > move on to the next one.
>
> To work around potential intermixed output, I rewrite the normal
> coccicheck arguments that uses --jobs to launch $nproc many spatch
> instances with -max and -index. I will switch back to using --jobs and
> see if I still get corrupted patches... (It's been a while since I
> created this alternative workflow.)

If you are using --max and --index, it seems that you are not using
parmap.  That should all work fine now.

julia