Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list

[Florian Westphal <fw@strlen.de>]

Kyuwon Shim <kyuwon.shim@alliedtelesis.co.nz> wrote:
> The issue "core dumped" occurred  from
> ulogd_unregister_fd(). One of the processes is unlink
> from list and remove, but some struct 'pi' values
> freed without ulogd_unregister_fd().
> Unlink process needs to access the previous pointer
> value of struct 'pi', but it was already freed.
> 
> Therefore, the free() process moved location
> after finishing all ulogd_unregister_fd().

I don't understand this patch.

llist_for_each_entry_safe() doesn't dereference 'pi' after its free'd.

Where does this deref happen?  Can you share a backtrace?

> +		}
> +	}
> +
> +	llist_for_each_entry(stack, &ulogd_pi_stacks, stack_list) {
> +		llist_for_each_entry_safe(pi, npi, &stack->list, list) {
>  			free(pi);

Perhaps there should be a 'llist_del' before pi gets free'd instead?