Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault running spicy-driver using %context / self.context() #1311

Closed
awelzel opened this issue Dec 4, 2022 · 1 comment · Fixed by #1312
Closed

Segmentation fault running spicy-driver using %context / self.context() #1311

awelzel opened this issue Dec 4, 2022 · 1 comment · Fixed by #1312
Assignees

Comments

@awelzel
Copy link
Contributor

awelzel commented Dec 4, 2022

Observing a segfault with the following parser when running it through spicy-driver:

$ cat context-crash.spicy 
module Test;

type State = tuple<ssl_requested: uint8>;

type X = unit {
  val: bytes &size=3 {
    if ( self.val == b"ssl" )
        self.context().ssl_requested = 1;
  }
};

public type Y = unit {
  %context = State;
  x: X;
};
$ docker pull zeekurity/spicy:latest                                                               
latest: Pulling from zeekurity/spicy                                                                                                                                    
Digest: sha256:0bab492a0e2a68d05cf69c2aa56f2e9c56a7f66162c3acf280acc665593d4254                                                                                         
Status: Image is up to date for zeekurity/spicy:latest                            
docker.io/zeekurity/spicy:latest                                                    
$ docker run -v $(pwd):/src -w /src  -it --rm  zeekurity/spicy-dev:latest
# spicyc --version
spicyc v1.6.0-dev.118 (258565a4)
# spicy-driver ./context-crash.spicy
Segmentation fault (core dumped)

Maybe self.context() isn't allowed in non-public types, but the segfault does not help to figure out what I'm doing wrong :-)

@awelzel awelzel changed the title Segmentation fault using %context / self.context() Segmentation fault running spicy-driver using %context / self.context() Dec 4, 2022
@bbannier
Copy link
Member

bbannier commented Dec 4, 2022

The issue here is that we are missing a validation1. The call to ssl.context() is invalid in X since it does not have a context, and we should reject the code (once a context is added, there will be an additionl validation kicking in that ensures that the unit with the context is public).

The workaround here is to make sure that every unit which makes use of self.context() also declares a context.

Footnotes

  1. There is an assertion in the code that we can resolve the type of the context. This check is not triggered in a release build and we instead attempt to pull a value out of an unset optional which triggers the segfault.

@bbannier bbannier self-assigned this Dec 4, 2022
bbannier added a commit that referenced this issue Dec 4, 2022
When declaring `context` methods we previously would `assert` that any
unit using `self.context()` actually would declare a `%context`. This is
not true and lead to segfaults in release builds which do not execute
code in `assert`.

This patch adds new validation which rejects uses of `self.context()` in
units which did not declare a context. Since validation happens after
declaration of the `context` methods has executed (the part which
previously had the `assert`), we return a dummy type (a void) for such
units, and then reject the code later.

Closes #1311.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants