| Message ID | 20180103093513.8752-1-olof.sivertsson@zenterio.com |
|---|---|
| State | Accepted |
| Delegated to: | Hans Dedecker |
| Headers | show |
| Series | [LEDE-DEV,netifd] proto: allow dumping protocol handlers without config_params | expand |
On Wed, Jan 3, 2018 at 10:35 AM, Olof Sivertsson <osivertsson@gmail.com> wrote: > When ubus invokes proto_dump_handlers, and a struct proto_handler has > been added with a NULL config_params, a segmentation fault occurs. > > Avoid this segmentation fault by checking for a NULL config_params > before further access. Hi, I'm unable to reproduce the reported netifd crash by using a proto shell handler having no proto_init_config function. Looking into the code the proto_handler config_params parameter is always assigned the proto shell handler config pointer in the function proto_shell_add_handler; afaict the config_parameter is not altered further in the code. Can you describe how you triggered the crash using a proto shell handler implementation ? Hans > > Signed-off-by: Olof Sivertsson <olof.sivertsson@zenterio.com> > --- > proto.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/proto.c b/proto.c > index 9eb31c5..6047735 100644 > --- a/proto.c > +++ b/proto.c > @@ -591,7 +591,7 @@ proto_dump_handlers(struct blob_buf *b) > void *v; > > c = blobmsg_open_table(b, p->name); > - if (p->config_params->validate) { > + if (p->config_params && p->config_params->validate) { > int i; > > v = blobmsg_open_table(b, "validate"); > -- > 2.15.1 > > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev
Hi, > I'm unable to reproduce the reported netifd crash by using a proto > shell handler having no proto_init_config function. > Looking into the code the proto_handler config_params parameter is > always assigned the proto shell handler config pointer in the function > proto_shell_add_handler; afaict the config_parameter is not altered > further in the code. > Can you describe how you triggered the crash using a proto shell > handler implementation ? That being said, I still think that the change is sensible. Even if the root cause turns out to be something else. ~ Jo
On Thu, Jan 4, 2018 at 9:01 AM, Olof Sivertsson <osivertsson@gmail.com> wrote: > Hi Hans, > >> >> I'm unable to reproduce the reported netifd crash by using a proto >> shell handler having no proto_init_config function. >> Looking into the code the proto_handler config_params parameter is >> always assigned the proto shell handler config pointer in the function >> proto_shell_add_handler; afaict the config_parameter is not altered >> further in the code. >> Can you describe how you triggered the crash using a proto shell >> handler implementation ? > > > Sorry for not giving the full background. > > Our customers have security requirements that makes shell-based protocol > handlers a hard sell. > We have instead written a custom non-shell DHCP protocol handler, inspired > by proto-static.c, and with no use for config_params it was set to NULL. > > It is working well, and the case with config_params = NULL seems to be > handled elsewhere in the code, e.g. in config.c. > Therefore we assumed that it should be handled everywhere, and hence this > patch. Thanks for giving the background; the patch makes definitely sense as config_params is checked for NULL in other places as you point out. I was just wondering what I was missing since I could not reproduce the issue. Hans > > -Olof Sivertsson >
diff --git a/proto.c b/proto.c index 9eb31c5..6047735 100644 --- a/proto.c +++ b/proto.c @@ -591,7 +591,7 @@ proto_dump_handlers(struct blob_buf *b) void *v; c = blobmsg_open_table(b, p->name); - if (p->config_params->validate) { + if (p->config_params && p->config_params->validate) { int i; v = blobmsg_open_table(b, "validate");
When ubus invokes proto_dump_handlers, and a struct proto_handler has been added with a NULL config_params, a segmentation fault occurs. Avoid this segmentation fault by checking for a NULL config_params before further access. Signed-off-by: Olof Sivertsson <olof.sivertsson@zenterio.com> --- proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)