| Message ID | 20190721001406.23785-2-fw@strlen.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | fix crash bug during rule restore | expand |
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?
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.
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.
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:
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(+)