From:   Thomas Zimmermann <tzimmermann@suse.de>
To:     deller@gmx.de, geert+renesas@glider.be, timur@kernel.org,
        rdunlap@infradead.org, paulus@samba.org, benh@kernel.crashing.org,
        linux@armlinux.org.uk, pjones@redhat.com, adaplas@gmail.com,
        s.hauer@pengutronix.de, shawnguo@kernel.org, mbroemme@libmpq.org,
        thomas@winischhofer.net, James.Bottomley@HansenPartnership.com,
        sudipm.mukherjee@gmail.com, teddy.wang@siliconmotion.com,
        corbet@lwn.net
Cc:     linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org,
        Thomas Zimmermann <tzimmermann@suse.de>
Subject: [PATCH v2 098/101] staging/sm750fb: Release g_settings in module-exit function
Date:   Thu,  9 Mar 2023 17:01:58 +0100
Message-Id: <20230309160201.5163-99-tzimmermann@suse.de>
In-Reply-To: <20230309160201.5163-1-tzimmermann@suse.de>
References: <20230309160201.5163-1-tzimmermann@suse.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Free g_settings in module-exit function for symmetry with its
allocation in module-init function. Fixes a possible undefined
dereference of the pointer.

The string g_settings is initialized ifrom within the module-init
function lynxfb_init() and used from within the PCI probe function.
It is later freed in the PCI device's remove function. Probing another
PCI device afterwards accesses g_settings in an undefined state.

Fix this by freeing g_settings in lynxfb_exit(). Also streamline the
code that creates g_settings in the first place.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/staging/sm750fb/sm750.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index effc7fcc3703..fa8ae20bb688 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -1093,13 +1093,12 @@ static void lynxfb_pci_remove(struct pci_dev *pdev)
 
 	iounmap(sm750_dev->pvReg);
 	iounmap(sm750_dev->pvMem);
-	kfree(g_settings);
 }
 
 static int __init lynxfb_setup(char *options)
 {
-	int len;
-	char *opt, *tmp;
+	size_t len;
+	char *opt, *outbuf;
 
 	if (!options || !*options) {
 		pr_warn("no options.\n");
@@ -1109,11 +1108,10 @@ static int __init lynxfb_setup(char *options)
 	pr_info("options:%s\n", options);
 
 	len = strlen(options) + 1;
-	g_settings = kzalloc(len, GFP_KERNEL);
-	if (!g_settings)
+	outbuf = kzalloc(len, GFP_KERNEL);
+	if (!outbuf)
 		return -ENOMEM;
-
-	tmp = g_settings;
+	g_settings = outbuf;
 
 	/*
 	 * Notes:
@@ -1133,12 +1131,11 @@ static int __init lynxfb_setup(char *options)
 		} else if (!strncmp(opt, "dual", strlen("dual"))) {
 			g_dualview = 1;
 		} else {
-			strcat(tmp, opt);
-			tmp += strlen(opt);
-			if (options)
-				*tmp++ = ':';
-			else
-				*tmp++ = 0;
+			if (outbuf != g_settings)
+				*outbuf++ = ':'; // add separator
+			len = strlen(opt);
+			memcpy(outbuf, opt, len);
+			outbuf += len;
 		}
 	}
 
@@ -1186,6 +1183,7 @@ module_init(lynxfb_init);
 static void __exit lynxfb_exit(void)
 {
 	pci_unregister_driver(&lynxfb_driver);
+	kfree(g_settings);
 }
 module_exit(lynxfb_exit);
 
-- 
2.39.2