[nft,1/3] src: erec: fall back to internal location if its null
diff mbox series

Message ID 20190721001406.23785-2-fw@strlen.de
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series
  • fix crash bug during rule restore
Related show

Commit Message

Florian Westphal July 21, 2019, 12:14 a.m. UTC
This should never happen (we should pass valid locations to the error
reporting functions), but in case we screw up we will segfault during
error reporting.

cat crash
table inet filter {
}
table inet filter {
      chain test {
        counter
    }
}
"nft -f crash" Now reports:
internal:0:0-0: Error: No such file or directory

... which is both bogus and useless, but better than crashing.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/erec.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Pablo Neira Ayuso July 21, 2019, 6:46 p.m. UTC | #1
On Sun, Jul 21, 2019 at 02:14:05AM +0200, Florian Westphal wrote:
> This should never happen (we should pass valid locations to the error
> reporting functions), but in case we screw up we will segfault during
> error reporting.
> 
> cat crash
> table inet filter {
> }
> table inet filter {
>       chain test {
>         counter
>     }
> }
> "nft -f crash" Now reports:
> internal:0:0-0: Error: No such file or directory
> 
> ... which is both bogus and useless, but better than crashing.

This should not ever happen, right?
Florian Westphal July 21, 2019, 6:50 p.m. UTC | #2
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sun, Jul 21, 2019 at 02:14:05AM +0200, Florian Westphal wrote:
> > This should never happen (we should pass valid locations to the error
> > reporting functions), but in case we screw up we will segfault during
> > error reporting.
> > 
> > cat crash
> > table inet filter {
> > }
> > table inet filter {
> >       chain test {
> >         counter
> >     }
> > }
> > "nft -f crash" Now reports:
> > internal:0:0-0: Error: No such file or directory
> > 
> > ... which is both bogus and useless, but better than crashing.
> 
> This should not ever happen, right?

It happens with current master plus above file.
Pablo Neira Ayuso July 23, 2019, 7:19 p.m. UTC | #3
On Sun, Jul 21, 2019 at 02:14:05AM +0200, Florian Westphal wrote:
> This should never happen (we should pass valid locations to the error
> reporting functions), but in case we screw up we will segfault during
> error reporting.
> 
> cat crash
> table inet filter {
> }
> table inet filter {
>       chain test {
>         counter
>     }
> }
> "nft -f crash" Now reports:
> internal:0:0-0: Error: No such file or directory
> 
> ... which is both bogus and useless, but better than crashing.

I'd suggest we add BUG() here, so we catch missing location
information via indesc == NULL. So we can fix the lack of it,
otherwise users will rely on internal, which is very limited.

Thanks.

Patch
diff mbox series

diff --git a/src/erec.c b/src/erec.c
index c550a596b38c..28197924a82c 100644
--- a/src/erec.c
+++ b/src/erec.c
@@ -92,6 +92,9 @@  void erec_print(struct output_ctx *octx, const struct error_record *erec,
 	FILE *f;
 	int l;
 
+	if (!indesc)
+		indesc = &internal_indesc;
+
 	switch (indesc->type) {
 	case INDESC_BUFFER:
 	case INDESC_CLI: