Re: [PATCH] hysdn: fix to a race condition in put_log_buffer

[Sergei Shtylyov]

Hello!

On 08/03/2017 02:33 PM, Anton Volkov wrote:

> The synchronization type that was used earlier to guard the loop that
> deletes unused log buffers may lead to a situation that prevents any
> thread from going through the loop.
>
> The patch deletes previously used synchronization mechanism and moves
> the loop under the spin_lock so the similar cases won't be feasible in
> the future.
>
> Found by by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Volkov <avolkov@xxxxxxxxx>
> ---
>   drivers/isdn/hysdn/hysdn_proclog.c | 27 ++++++++++++---------------
>   1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/isdn/hysdn/hysdn_proclog.c b/drivers/isdn/hysdn/hysdn_proclog.c
> index 7b5fd8f..b152c6c 100644
> --- a/drivers/isdn/hysdn/hysdn_proclog.c
> +++ b/drivers/isdn/hysdn/hysdn_proclog.c
[...]
>   	else
>   		pd->log_tail->next = ib;	/* follows existing messages */
>   	pd->log_tail = ib;	/* new tail */
> -	i = pd->del_lock++;	/* get lock state */
> -	spin_unlock_irqrestore(&card->hysdn_lock, flags);
>   
>   	/* delete old entrys */
> -	if (!i)
> -		while (pd->log_head->next) {
> -			if ((pd->log_head->usage_cnt <= 0) &&
> -			    (pd->log_head->next->usage_cnt <= 0)) {
> -				ib = pd->log_head;
> -				pd->log_head = pd->log_head->next;
> -				kfree(ib);
> -			} else
> -				break;
> -		}		/* pd->log_head->next */
> -	pd->del_lock--;		/* release lock level */
> +	while (pd->log_head->next) {
> +		if ((pd->log_head->usage_cnt <= 0) &&
> +		    (pd->log_head->next->usage_cnt <= 0)) {
> +			ib = pd->log_head;
> +			pd->log_head = pd->log_head->next;
> +			kfree(ib);
> +		} else
> +			break;
> +	}		/* pd->log_head->next */

   Need {} in the *else* branch as *if* had them.

[...]

MBR, Sergei