Re: [PATCH 1/2] xhci: Fix invalid loop check in xhci_free_tt_info()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 01, 2012 at 10:06:23AM +0200, Takashi Iwai wrote:
> xhci_free_tt_info() may access the invalid memory when it removes the
> last entry but the list is not empty.  Then tt_next reaches to the
> list head but it still tries to check the tt_info of that entry.
> 
> This patch fixes the bug and cleans up the messy code by rewriting
> with a simple list_for_each_entry_safe().
> 
> Cc: <stable@xxxxxxxxxxxxxxx>
> Reviewed-by: Oliver Neukum <oneukum@xxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>

Ok, this looks correct.  I'll send it off to Greg in my next 3.5 pull
request.

Sarah Sharp

> ---
>  drivers/usb/host/xhci-mem.c |   38 +++++++++-----------------------------
>  1 file changed, 9 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index ec4338e..4e1da7f 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -793,10 +793,9 @@ static void xhci_free_tt_info(struct xhci_hcd *xhci,
>  		struct xhci_virt_device *virt_dev,
>  		int slot_id)
>  {
> -	struct list_head *tt;
>  	struct list_head *tt_list_head;
> -	struct list_head *tt_next;
> -	struct xhci_tt_bw_info *tt_info;
> +	struct xhci_tt_bw_info *tt_info, *next;
> +	bool slot_found = false;
>  
>  	/* If the device never made it past the Set Address stage,
>  	 * it may not have the real_port set correctly.
> @@ -808,34 +807,15 @@ static void xhci_free_tt_info(struct xhci_hcd *xhci,
>  	}
>  
>  	tt_list_head = &(xhci->rh_bw[virt_dev->real_port - 1].tts);
> -	if (list_empty(tt_list_head))
> -		return;
> -
> -	list_for_each(tt, tt_list_head) {
> -		tt_info = list_entry(tt, struct xhci_tt_bw_info, tt_list);
> -		if (tt_info->slot_id == slot_id)
> +	list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
> +		/* Multi-TT hubs will have more than one entry */
> +		if (tt_info->slot_id == slot_id) {
> +			slot_found = true;
> +			list_del(&tt_info->tt_list);
> +			kfree(tt_info);
> +		} else if (slot_found)
>  			break;
>  	}
> -	/* Cautionary measure in case the hub was disconnected before we
> -	 * stored the TT information.
> -	 */
> -	if (tt_info->slot_id != slot_id)
> -		return;
> -
> -	tt_next = tt->next;
> -	tt_info = list_entry(tt, struct xhci_tt_bw_info,
> -			tt_list);
> -	/* Multi-TT hubs will have more than one entry */
> -	do {
> -		list_del(tt);
> -		kfree(tt_info);
> -		tt = tt_next;
> -		if (list_empty(tt_list_head))
> -			break;
> -		tt_next = tt->next;
> -		tt_info = list_entry(tt, struct xhci_tt_bw_info,
> -				tt_list);
> -	} while (tt_info->slot_id == slot_id);
>  }
>  
>  int xhci_alloc_tt_info(struct xhci_hcd *xhci,
> -- 
> 1.7.9.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux