linux-kernel - RE: [PATCH] Drivers: hv: vmbus: serialize Offer and Rescind offer processing

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] [day] [month] [year] [list]

Date:	Sat, 17 Jan 2015 01:27:17 +0000
From:	KY Srinivasan <kys@...rosoft.com>
To:	Vitaly Kuznetsov <vkuznets@...hat.com>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>
CC:	Haiyang Zhang <haiyangz@...rosoft.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Dexuan Cui <decui@...rosoft.com>,
	Jason Wang <jasowang@...hat.com>,
	Radim Krčmář <rkrcmar@...hat.com>
Subject: RE: [PATCH] Drivers: hv: vmbus: serialize Offer and Rescind offer
 processing



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@...hat.com]
> Sent: Friday, January 16, 2015 5:46 AM
> To: KY Srinivasan; devel@...uxdriverproject.org
> Cc: Haiyang Zhang; linux-kernel@...r.kernel.org; Dexuan Cui; Jason Wang;
> Radim Krčmář
> Subject: [PATCH] Drivers: hv: vmbus: serialize Offer and Rescind offer
> processing
> 
> Commit 4b2f9abea52a ("staging: hv: convert channel_mgmt.c to not call
> osd_schedule_callback")' was written under an assumption that we never
> receive Rescind offer while we're still processing the initial Offer request.
> However, the issue we fixed in 04a258c162a8 could be caused by this
> assumption not always being true.
> 
> In particular, we need to protect against the following:
> 1) Receiving a Rescind offer after we do queue_work() for processing an
> Offer
>    request and before we actually enter vmbus_process_offer(). work.func
> points
>    to vmbus_process_offer() at this moment and in vmbus_onoffer_rescind()
> we do
>    another queue_work() without a check so we'll enter
> vmbus_process_offer()
>    twice.
> 2) Receiving a Rescind offer after we enter vmbus_process_offer() and
>    especially after we set >state = CHANNEL_OPEN_STATE. Many things can
> go
>    wrong in that case, e.g. we can call free_channel() while we're still using
>    it.
> 
> Implement the required protection by changing work->func at the very end
> of
> vmbus_process_offer() and checking work->func in
> vmbus_onoffer_rescind().
> pr_warn here serves only one purpose, it shows us the collision between
> Offer and Rescind offer actually happened. In case the assumption in
> 4b2f9abea52af3 was right and such collision is impossible this commit changes
> nothing.

It is clear that an offer that has not been made cannot be rescinded. So if a rescind were to come while 
we are in the process of handling the offer, we should deal with the rescind as well after the processing of the offer is done.
Your patch does not do this; essentially we could miss handling a rescind message. We could tweak your patch to
deal with this case. If a rescind message comes in while we are still processing the offer, we should process the
rescind message in the same context. We can add this check at the end of the function that processes the offer.
If the offer is processed fully when the rescind message comes in then we can process the rescind request in its own
work context.

> 
> I also fix one additional issue with this patch: vmbus_device_create() result is
> not being checked in vmbus_process_offer() and it can fail if kzalloc() fails.

Please have this fix in a separate patch.

Thanks,

K. Y