From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-1560247-1517064508-2-2385250471080111948 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, FREEMAIL_FROM 0.001, RCVD_IN_DNSWL_NONE -0.0001, RCVD_IN_MSPIKE_H2 -0.001, SPF_PASS -0.001, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.85.128.193', Host='mail-wr0-f193.google.com', Country='US', FromHeader='com', MailFrom='com' X-Spam-charsets: from='utf-8', plain='utf-8' X-Attached: signature.asc X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: pali.rohar@gmail.com ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1517064507; b=bYbdIheQUuOhmsjWKN0OcthJQozv9v87x64/DNdFe6CYPQH LsC7a1PJWcN6TX+Ne7KFKPtHAywmsYPMTsPW7zSHgF8Cyt2AzqzTGU6g9QQBprFV hlRhkMA0U5e9gohf4dDpuGIc+CAEMoXaIPKvM5/60EIFk9WZ+2Gv8+EkDcukjLLF MCSP0+Af2sQpP7QFaL30TqPdj8+nTQepowZwArnC16wkv3X6B6v8dHZi7cKs6PLW e6DyTZMtT8tRyhzbM3V+kNilA5lLcHQDGzFMJSh97wNo4mGGfTKNskwTZZSdZfYd GPSAagj03oP0qAFAGacTdj1xqY3ILsuX567jbiQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=date:from:to:cc:subject:message-id :references:mime-version:content-type:in-reply-to; s=arctest; t= 1517064507; bh=en+yUw92LmQtapnUL70D7t8hx7Dy6W5hM3uoqv5Xh2o=; b=P 2jRaVMQ/O9QvVs+BvOY+27i1iabCFj0SWzKWbccfnTTM8pr1L0TaJcVValqVsDyo F1x2ROk2wBpSylhYWyl38gTpBmQGj9CPAI1+2bAsS0jeQtpNAVDTNwo8rCYzGpv9 ewWnOizfGXLxiNNiRO8nZ+iZ0nlOBIIQzKBiWPMZrhJn7yA1f//LlOGBtXq2ctcs mmYg5iKH+8LY8qVf0Hkll/u4TKUTxy4PRQInXfXJUOK5EE1fvaaxyeBaM9pNJDZY BF/tXtQgVPASSX4KW8TNP7yK9GZux1OhTS88oz1VNbBblwZci0MQ163Co2aahdUS 6jGN3b/MgkTx3lVLJxa1A== ARC-Authentication-Results: i=1; mx3.messagingengine.com; arc=none (no signatures found); dkim=pass (2048-bit rsa key sha256) header.d=gmail.com header.i=@gmail.com header.b=RXBoHPgR x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=pass (p=none,d=none) header.from=gmail.com; iprev=pass policy.iprev=209.85.128.193 (mail-wr0-f193.google.com); spf=pass smtp.mailfrom=pali.rohar@gmail.com smtp.helo=mail-wr0-f193.google.com; x-aligned-from=pass; x-google-dkim=pass (2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=HPQbBWGV; x-ptr=pass x-ptr-helo=mail-wr0-f193.google.com x-ptr-lookup=mail-wr0-f193.google.com; x-return-mx=pass smtp.domain=gmail.com smtp.result=pass smtp_is_org_domain=yes header.domain=gmail.com header.result=pass header_is_org_domain=yes; x-tls=pass version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128 Authentication-Results: mx3.messagingengine.com; arc=none (no signatures found); dkim=pass (2048-bit rsa key sha256) header.d=gmail.com header.i=@gmail.com header.b=RXBoHPgR x-bits=2048 x-keytype=rsa x-algorithm=sha256 x-selector=20161025; dmarc=pass (p=none,d=none) header.from=gmail.com; iprev=pass policy.iprev=209.85.128.193 (mail-wr0-f193.google.com); spf=pass smtp.mailfrom=pali.rohar@gmail.com smtp.helo=mail-wr0-f193.google.com; x-aligned-from=pass; x-google-dkim=pass (2048-bit rsa key) header.d=1e100.net header.i=@1e100.net header.b=HPQbBWGV; x-ptr=pass x-ptr-helo=mail-wr0-f193.google.com x-ptr-lookup=mail-wr0-f193.google.com; x-return-mx=pass smtp.domain=gmail.com smtp.result=pass smtp_is_org_domain=yes header.domain=gmail.com header.result=pass header_is_org_domain=yes; x-tls=pass version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128 X-Google-Smtp-Source: AH8x227uL3xXrfvv3a1PPjoddwnORVcxOcqsfJIqI0tKDAMIw5cv6eVInj6H7OKa+N6LU8MZGeWJMg== Date: Sat, 27 Jan 2018 15:48:23 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Mario Limonciello Cc: dvhart@infradead.org, Andy Shevchenko , LKML , platform-driver-x86@vger.kernel.org, Andy Lutomirski , quasisec@google.com, rjw@rjwysocki.net, mjg59@google.com, hch@lst.de, Greg KH , Alan Cox Subject: Re: [PATCH v12 10/16] platform/x86: dell-smbios: Introduce dispatcher for SMM calls Message-ID: <20180127144823.innndhmtaus3k7op@pali> References: <19848e4dec380f91efd1536bbfad092050d232d2.1509561822.git.mario.limonciello@dell.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vx4leyovcl24qgzk" Content-Disposition: inline In-Reply-To: <19848e4dec380f91efd1536bbfad092050d232d2.1509561822.git.mario.limonciello@dell.com> User-Agent: NeoMutt/20170113 (1.7.2) X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: --vx4leyovcl24qgzk Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi! On Wednesday 01 November 2017 14:25:31 Mario Limonciello wrote: > This splits up the dell-smbios driver into two drivers: > * dell-smbios > * dell-smbios-smm >=20 > dell-smbios can operate with multiple different dispatcher drivers to > perform SMBIOS operations. >=20 > Also modify the interface that dell-laptop and dell-wmi use align to this > model more closely. Rather than a single global buffer being allocated > for all drivers, each driver will allocate and be responsible for it's own > buffer. The pointer will be passed to the calling function and each > dispatcher driver will then internally copy it to the proper location to > perform it's call. >=20 > Add defines for calls used by these methods in the dell-smbios.h header > for tracking purposes. >=20 > Signed-off-by: Mario Limonciello > Reviewed-by: Edward O'Callaghan > --- =2E.. > @@ -85,6 +73,7 @@ static struct platform_driver platform_driver =3D { > } > }; > =20 > +static struct calling_interface_buffer *buffer; > static struct platform_device *platform_device; > static struct backlight_device *dell_backlight_device; > static struct rfkill *wifi_rfkill; > @@ -283,6 +272,27 @@ static const struct dmi_system_id dell_quirks[] __in= itconst =3D { > { } > }; > =20 > +void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3) > +{ > + memset(buffer, 0, sizeof(struct calling_interface_buffer)); > + buffer->input[0] =3D arg0; > + buffer->input[1] =3D arg1; > + buffer->input[2] =3D arg2; > + buffer->input[3] =3D arg3; > +} > + > +int dell_send_request(u16 class, u16 select) > +{ > + int ret; > + > + buffer->cmd_class =3D class; > + buffer->cmd_select =3D select; > + ret =3D dell_smbios_call(buffer); > + if (ret !=3D 0) > + return ret; > + return dell_smbios_error(buffer->output[0]); > +} > + > /* > * Derived from information in smbios-wireless-ctl: > * =2E.. > @@ -413,20 +422,16 @@ static int dell_rfkill_set(void *data, bool blocked) > int status; > int ret; > =20 > - buffer =3D dell_smbios_get_buffer(); > - > - dell_smbios_send_request(17, 11); > - ret =3D buffer->output[0]; > + dell_set_arguments(0, 0, 0, 0); > + ret =3D dell_send_request(CLASS_INFO, SELECT_RFKILL); > + if (ret) > + return ret; > status =3D buffer->output[1]; Now I'm looking at this patch again and I think it introduced a new race condition. Prior this patch, dell_smbios_get_buffer() acquired mutex and returned pointer to buffer which caller used and then released. Now buffer is allocated at module load time and is shared for all functions in this module. And hen is used, there is no mutex protection for it. First call is to dell_set_arguments() which modifies that shared buffer and then dell_send_request() is used to modify and pass buffer to another function dell_smbios_call(). And function below dell_update_rfkill() is called work queue which also modifies buffer by dell_set_arguments() function. Therefore it looks like when dell_update_rfkill() is scheduled at the same time as dell_rfkill_set() is executed, then both functions overwrite one shared buffer and something unexpected would be sent to SMM. > @@ -613,46 +596,36 @@ static const struct file_operations dell_debugfs_fo= ps =3D { > =20 > static void dell_update_rfkill(struct work_struct *ignored) > { > - struct calling_interface_buffer *buffer; > int hwswitch =3D 0; > int status; > int ret; > =20 > - buffer =3D dell_smbios_get_buffer(); > - > - dell_smbios_send_request(17, 11); > - ret =3D buffer->output[0]; > + dell_set_arguments(0, 0, 0, 0); > + ret =3D dell_send_request(CLASS_INFO, SELECT_RFKILL); > status =3D buffer->output[1]; --=20 Pali Roh=C3=A1r pali.rohar@gmail.com --vx4leyovcl24qgzk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABECAB0WIQS4VrIQdKium2krgIWL8Mk9A+RDUgUCWmyRNAAKCRCL8Mk9A+RD UrSeAKDGwNWrSSKCTakN2QxCtTpSJDv42gCeOH+RZGItnIiWfCWoD1VTIhvDiyY= =kteA -----END PGP SIGNATURE----- --vx4leyovcl24qgzk--