Re: [iptables PATCH 2/2] nfnl_osf: Improve error handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 11, 2020 at 01:31:12PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Sat, May 09, 2020 at 07:28:07PM +0200, Pablo Neira Ayuso wrote:
> > On Sat, May 09, 2020 at 01:52:00PM +0200, Phil Sutter wrote:
> > > For some error cases, no log message was created - hence apart from the
> > > return code there was no indication of failing execution.
> > > 
> > > When loading a line fails, don't abort but continue with the remaining
> > > file contents. The current pf.os file in this repository serves as
> > > proof-of-concept: Loading all entries succeeds, but when deleting, lines
> > > 700, 701 and 704 return ENOENT. Not continuing means the remaining
> > > entries are not cleared.
> > 
> > Did you look at why are these lines returning ENOENT?
> 
> If I understand the code right, line 700 is a duplicate of line 698, 701
> of 699 and 704 of 702. This is because 'W*' parses identical to 'W0' and
> in right-hand side only the first three text fields (genre, version and
> subtype) are relevant - the rest is ignored.

I see, in the userspace parser, W0 and W* are being handled as
OSF_WSS_PLAIN.

> When adding, this doesn't become visible because flag NLM_F_EXCL is not
> specified. If it is, kernel returns EEXISTS for those lines.

In the kernel, the struct nf_osf_user_finger is used as key to
identify each line, given they are identical.

So it looks like this EEXIST has been there since the beginning.

This patchset LGTM, it's just that the user might get confused if it
see errors when using this tool, probably turning this into a warning
is fine.

Or at least, include this information in the commit message so this
does not get lost :-)



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux