Hi Christian You are right on that part of obj-staged is set to NULL in add_fence, So my following question will be why we kfree(obj->staged) in reserve_shared() if staged is always NULL in that point ? Thanks /Monk -----Original Message----- From: Christian König [mailto:ckoenig.leichtzumerken@xxxxxxxxx] Sent: 2018年2月28日 16:27 To: Liu, Monk <Monk.Liu@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx Subject: Re: [PATCH] dma-buf/reservation: shouldn't kfree staged when slot available Am 28.02.2018 um 07:44 schrieb Monk Liu: > under below scenario the obj->fence would refer to a wild pointer: > > 1,call reservation_object_reserved_shared > 2,call reservation_object_add_shared_fence > 3,call reservation_object_reserved_shared > 4,call reservation_object_add_shared_fence > > in step 1, staged is allocated, > > in step 2, code path will go reservation_object_add_shared_replace() > and obj->fence would be assigned as staged (through RCU_INIT_POINTER) > > in step 3, obj->staged will be freed(by simple kfree), which make > obj->fence point to a wild pointer... Well that explanation is still nonsense. See reservation_object_add_shared_fence: > obj->staged = NULL; Among the first things reservation_object_add_shared_fence() does is it sets obj->staged to NULL. So step 3 will not free anything and we never have a wild pointer. Regards, Christian. > > in step 4, code path will go reservation_object_add_shared_inplace() > and inside it the @fobj (which equals to @obj->staged, set by above steps) > is already a wild pointer > > should remov the kfree on staged in reservation_object_reserve_shared() > > Change-Id: If7c01f1b4be3d3d8a81efa90216841f79ab1fc1c > Signed-off-by: Monk Liu <Monk.Liu@xxxxxxx> > --- > drivers/dma-buf/reservation.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index 375de41..b473ccc 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -74,12 +74,9 @@ int reservation_object_reserve_shared(struct reservation_object *obj) > old = reservation_object_get_list(obj); > > if (old && old->shared_max) { > - if (old->shared_count < old->shared_max) { > - /* perform an in-place update */ > - kfree(obj->staged); > - obj->staged = NULL; > + if (old->shared_count < old->shared_max) > return 0; > - } else > + else > max = old->shared_max * 2; > } else > max = 4; _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
