linux-kernel - RE: [PATCH] Drivers: hv: kvp/vss: Avoid accessing a ringbuffer not initialized yet

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]

Message-ID:
 <SA1PR21MB1317A021B6C5D552B38C4368BF562@SA1PR21MB1317.namprd21.prod.outlook.com>
Date: Fri, 1 Nov 2024 00:16:39 +0000
From: Dexuan Cui <decui@...rosoft.com>
To: Michael Kelley <mhklinux@...look.com>, KY Srinivasan <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Vitaly
 Kuznetsov <vkuznets@...hat.com>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, "open list:Hyper-V/Azure CORE AND DRIVERS"
	<linux-hyperv@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
CC: "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] Drivers: hv: kvp/vss: Avoid accessing a ringbuffer not
 initialized yet

> From: Michael Kelley <mhklinux@...look.com>
> Sent: Wednesday, October 30, 2024 5:12 PM
> [...]
> What do you think about this (compile tested only), which splits the
> "init" function into two parts for devices that have char devs? I'm
> trying to avoid adding yet another synchronization point by just
> doing the init operations in the right order -- i.e., don't create the
> user space /dev entry until the VMBus channel is ready.
> 
> Michael

Thanks, I think this works! This is a better fix.

> +	if (srv->util_init_transport) {
> +		ret = srv->util_init_transport();
> +		if (ret) {
> +			ret = -ENODEV;
IMO we don't need the line above, since the 'ret' from 
srv->util_init_transport()  is already a standard error code.

BTW, I noticed that the line "ret = -ENODEV;"
        if (srv->util_init) {
                ret = srv->util_init(srv);
                if (ret) {
                        ret = -ENODEV;
                        goto error1;
                }
        }
I think we don't really need that line, either. 
The existing 4 .util_init callbacks also already return a
standard error code. We can make a separate patch to clean
that up.

Thanks,
Dexuan