>  > I couldn't see a reason for a pf_tag_unref in the so_accept because
>  > the socket could be reused.
>
> don't we need an additional ref (aka tagname2tag or the like), not unref,
>  since the socket gets cloned?
>

if you want a diffrent tag on the resulting socket, then we have a
problem, because if you change the tag on the socket afterwards, you
unref the tag from the other socket, since I add this.

>  > +                     so->so_pftag = pf_tagname2tag(mtod(m, char *));
>  > +                     if(so->so_pftag == 0)
>  > +                     {
>  > +                             error = EINVAL; /*XXX*/
> why XXX?

I forgot to mentioned that. I don't think that EINVAL is the correct
errno for this case. pf_tagname2tag return 0 if the malloc faild or
the max number of tags is reached allready. I thought ENOMEM would be
better for example, but I wasn't sure, so this was my reminder...

>  > +
> the above chunk seems to be an accident

I can't believe that this happened. I read three times the diff.

> oh, what about ip6_output? :)
this evening after work :)

>> Nice, you probably want to keep the application/kernel tag name spaces
>> distinct though. Otherwise it would be easy for any local user/program
>> to mess with pf.conf generated tags and bypass filtering etc. It could
>> be as easy as adding a prefix ("APP_" ?) to all application generated
>> tags.

>actually you have a point here... sockets don't even require root.

That is true, my point is that to change the tags in the kernel is not
a nice way. A programm which set the tag "VPN1" and will get
"APP_VPN1" ?? This is not a good behavior, IMHO.

Reply via email to