* Konrad <[EMAIL PROTECTED]> [2008-03-07 15:01]:
> >  > 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.

no, not a different one.
consider this scenario:
s1 = socket(), bind(s1), assing tag "foo".
s2 = accept(s).
we now have s1 and s2 BOTH refer to tag "foo".
close(s2) -> tag_unref -> refcount 0 -> no more tag foo. kaboom.

> >  > +                     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...

hmm, yeah, ENOMEM might be better. not entirely certain tho.

> >  > +
> > the above chunk seems to be an accident
> I can't believe that this happened. I read three times the diff.

happens to all of us once in a while :)

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

cool.

> >> 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.

we might want to for the @ prefix canacar suggested. we might 
automagically add and strip it in the ioctls. it would require 
documentation to use "@foo" in pf instead of foo. requires some thought 
regarding the right semantics.
we do NOt want to change the way the current tags work form a user 
perspective. we don't want to fuck our users on upgrade :)

-- 
Henning Brauer, [EMAIL PROTECTED], [EMAIL PROTECTED]
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting - Hamburg & Amsterdam

Reply via email to