linux-kernel - RE: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Jan 2019 03:46:48 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     kimbrownkd <kimbrownkd@...il.com>,
        Michael Kelley <mikelley@...rosoft.com>,
        Long Li <longli@...rosoft.com>,
        Sasha Levin <Alexander.Levin@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show
 functions

> From: Kimberly Brown <kimbrownkd@...il.com>
> Sent: Monday, January 21, 2019 6:08 PM
> Subject: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
> 
> The channel level "_show" functions are vulnerable to race conditions.
> Add a mutex lock and unlock around the call to the channel level "_show"
> functions in vmbus_chan_attr_show().
> 
> This problem was discussed here:
> https://lkml.org/lkml/2018/10/18/830
> 
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1414,6 +1414,7 @@ static ssize_t vmbus_chan_attr_show(struct kobject
> *kobj,
>  		= container_of(attr, struct vmbus_chan_attribute, attr);
>  	const struct vmbus_channel *chan
>  		= container_of(kobj, struct vmbus_channel, kobj);
> +	ssize_t ret;
> 
>  	if (!attribute->show)
>  		return -EIO;
> @@ -1421,7 +1422,10 @@ static ssize_t vmbus_chan_attr_show(struct
> kobject *kobj,
>  	if (chan->state != CHANNEL_OPENED_STATE)
>  		return -EINVAL;
> 
> -	return attribute->show(chan, buf);
> +	mutex_lock(&vmbus_connection.channel_mutex);
> +	ret = attribute->show(chan, buf);
> +	mutex_unlock(&vmbus_connection.channel_mutex);
> +	return ret;
>  }

It looks this patch is already able to fix the original issue:
6712cc9c2211 ("vmbus: don't return values for uninitalized channels"),
as chan->state can't be CHANNEL_OPENED_STATE when
channel->ringbuffer_page is not set up yet, or has been freed.

Thanks,
-- Dexuan