Herbert,
On Mon, 2005-04-04 at 08:16, Herbert Xu wrote:
> Hi Jamal:
>
> On Mon, Apr 04, 2005 at 07:38:26AM -0400, jamal wrote:
> > Ok, heres an update.
> > +void km_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c);
> > +void km_state_notify(struct xfrm_state *x, struct km_event *c);
>
> No need for these prototypes since they're already in xfrm.h.
Good catch.
>
> > @@ -764,37 +778,60 @@
> > }
> > EXPORT_SYMBOL(xfrm_replay_advance);
> >
> > -static struct list_head xfrm_km_list = LIST_HEAD_INIT(xfrm_km_list);
> > -static DEFINE_RWLOCK(xfrm_km_lock);
>
> How about letting these guys stay where they are? The move was
> necessary before because the km_*_notify functions had to be called
> in this file but that's no longer the case.
>
Changed
- dont see what the harm was as they were in that patch though.
[..]
> Sorry, you need to hold x before the call to
> xfrm_state_add/xfrm_state_update as otherwise
> they can cause x to be freed.
>
> In general hold/put is only useful if
>
> 1) When you call hold you already have a reference to the object.
> 2) In between the hold/put you may free the object.
>
[..]
> Same problem as xfrm_user. We need the hold to occur before the
> add/update for it to be effective. In fact the original code was
> buggy too since it didn't hold a reference at all.
>
> Of course this is very unlikely to crash since it requires a
> small life time and some bad luck in getting preempted.
>
;-> Yes, indeed. I think its time for you to throw in the towel ;->
There was really not even a need for that hold given the likelihood of
anything like this happening. Anyways ive made this fix and heres the
updated patch.
cheers,
jamal
ipsec-event-take2-3
Description: Text document
|