* 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