From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2] gpiolib-acpi: Duplicate con_id string when adding it to the crs lookup list Date: Thu, 28 Apr 2016 15:30:56 +0300 Message-ID: <20160428123056.GC4329@intel.com> References: <1460563836-1899-1-git-send-email-ville.syrjala@linux.intel.com> <1461589279-8911-1-git-send-email-ville.syrjala@linux.intel.com> <20160428103333.GY4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mga04.intel.com ([192.55.52.120]:27422 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751411AbcD1MbH (ORCPT ); Thu, 28 Apr 2016 08:31:07 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Alexandre Courbot Cc: "linux-gpio@vger.kernel.org" , Dmitry Torokhov , Mika Westerberg , Linus Walleij , Stable On Thu, Apr 28, 2016 at 08:55:36PM +0900, Alexandre Courbot wrote: > On Thu, Apr 28, 2016 at 7:33 PM, Ville Syrj=E4l=E4 > wrote: > > On Thu, Apr 28, 2016 at 02:04:10PM +0900, Alexandre Courbot wrote: > >> On Mon, Apr 25, 2016 at 10:01 PM, = wrote: > >> > From: Ville Syrj=E4l=E4 > >> > > >> > Calling gpiod_get() from a module and then unloading the module = leads to an > >> > oops due to acpi_can_fallback_to_crs() storing the pointer to th= e passed > >> > 'con_id' string onto acpi_crs_lookup_list. The next guy to come = along will then > >> > try to access the string but the memory may now be gone with the= module. > >> > Make a copy of the passed string instead, and store the copy on = the list. > >> > > >> > BUG: unable to handle kernel paging request at ffffffffa03e7855 > >> > IP: [] strcmp+0x12/0x30 > >> > PGD 2a07067 PUD 2a08063 PMD 74720067 PTE 0 > >> > Oops: 0000 [#1] PREEMPT SMP > >> > Modules linked in: i915(+) drm_kms_helper drm intel_gtt snd_hda_= codec snd_hda_core i2c_algo_bit syscopya > >> > rea sysfillrect sysimgblt fb_sys_fops agpgart snd_soc_sst_bytcr_= rt5640 coretemp hwmon intel_rapl intel_soc_dts_thermal > >> > punit_atom_debug snd_soc_rt5640 snd_soc_rl6231 serio snd_intel_s= st_acpi snd_intel_sst_core video snd_soc_sst_mfld_platf > >> > orm snd_soc_sst_match backlight int3402_thermal processor_therma= l_device int3403_thermal int3400_thermal acpi_thermal_r > >> > el snd_soc_core intel_soc_dts_iosf int340x_thermal_zone snd_comp= ress i2c_hid hid snd_pcm snd_timer snd soundcore evdev > >> > sch_fq_codel efivarfs ipv6 autofs4 [last unloaded: drm] > >> > CPU: 2 PID: 3064 Comm: modprobe Tainted: G U W 4.6.0-= rc3-ffrd-ipvr+ #302 > >> > Hardware name: Intel Corp. VALLEYVIEW C0 PLATFORM/BYT-T FFD8, BI= OS BLAKFF81.X64.0088.R10.1403240443 FFD8 > >> > _X64_R_2014_13_1_00 03/24/2014 > >> > task: ffff8800701cd200 ti: ffff880070034000 task.ti: ffff8800700= 34000 > >> > RIP: 0010:[] [] strcmp+0x12= /0x30 > >> > RSP: 0000:ffff880070037748 EFLAGS: 00010286 > >> > RAX: 0000000080000000 RBX: ffff88007a342800 RCX: 000000000000000= 6 > >> > RDX: 0000000000000006 RSI: ffffffffa054f856 RDI: ffffffffa03e785= 6 > >> > RBP: ffff880070037748 R08: 0000000000000000 R09: 000000000000000= 1 > >> > R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa054f85= 5 > >> > R13: ffff88007281cae0 R14: 0000000000000010 R15: ffffffffffffffe= a > >> > FS: 00007faa51447700(0000) GS:ffff880079300000(0000) knlGS:0000= 000000000000 > >> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> > CR2: ffffffffa03e7855 CR3: 0000000041eba000 CR4: 00000000001006e= 0 > >> > Stack: > >> > ffff880070037770 ffffffff8136ad28 ffffffffa054f855 000000000000= 0000 > >> > ffff88007a0a2098 ffff8800700377e8 ffffffff8136852e ffff88007a34= 2800 > >> > 00000007700377a0 ffff8800700377a0 ffffffff81412442 70672d6c656e= 6170 > >> > Call Trace: > >> > [] acpi_can_fallback_to_crs+0x88/0x100 > >> > [] gpiod_get_index+0x25e/0x310 > >> > [] ? mipi_dsi_attach+0x22/0x30 > >> > [] gpiod_get+0x12/0x20 > >> > [] intel_dsi_init+0x421/0x480 [i915] > >> > [] intel_modeset_init+0x853/0x16b0 [i915] > >> > [] ? intel_setup_gmbus+0x214/0x260 [i915] > >> > [] i915_driver_load+0xdc8/0x19b0 [i915] > >> > [] ? _raw_spin_unlock_irqrestore+0x43/0x70 > >> > [] drm_dev_register+0xab/0xc0 [drm] > >> > [] drm_get_pci_dev+0x93/0x1f0 [drm] > >> > [] ? _raw_spin_unlock_irqrestore+0x43/0x70 > >> > [] i915_pci_probe+0x34/0x50 [i915] > >> > [] pci_device_probe+0x91/0x100 > >> > [] driver_probe_device+0x20a/0x2d0 > >> > [] __driver_attach+0x9e/0xb0 > >> > [] ? driver_probe_device+0x2d0/0x2d0 > >> > [] bus_for_each_dev+0x69/0xa0 > >> > [] driver_attach+0x1e/0x20 > >> > [] bus_add_driver+0x1c0/0x240 > >> > [] driver_register+0x60/0xe0 > >> > [] __pci_register_driver+0x60/0x70 > >> > [] drm_pci_init+0xe4/0x110 [drm] > >> > [] ? trace_hardirqs_on+0xe/0x10 > >> > [] ? 0xffffffffa02f1000 > >> > [] i915_init+0x94/0x9b [i915] > >> > [] do_one_initcall+0x8b/0x1c0 > >> > [] ? rcu_read_lock_sched_held+0x86/0x90 > >> > [] ? kmem_cache_alloc_trace+0x1f6/0x270 > >> > [] do_init_module+0x60/0x1dc > >> > [] load_module+0x1d0d/0x2390 > >> > [] ? __symbol_put+0x70/0x70 > >> > [] ? kernel_read_file+0x92/0x120 > >> > [] SYSC_finit_module+0xa4/0xb0 > >> > [] SyS_finit_module+0xe/0x10 > >> > [] do_syscall_64+0x63/0x350 > >> > [] entry_SYSCALL64_slow_path+0x25/0x25 > >> > Code: f7 48 8d 76 01 48 8d 52 01 0f b6 4e ff 84 c9 88 4a ff 75 e= d 5d c3 0f 1f 00 55 48 89 e5 eb 04 84 c0 > >> > 74 18 48 8d 7f 01 48 8d 76 01 <0f> b6 47 ff 3a 46 ff 74 eb 19 c= 0 83 c8 01 5d c3 31 c0 5d c3 66 > >> > RIP [] strcmp+0x12/0x30 > >> > RSP > >> > CR2: ffffffffa03e7855 > >> > > >> > v2: Make the copied con_id const > >> > > >> > Cc: Dmitry Torokhov > >> > Cc: Mika Westerberg > >> > Cc: Linus Walleij > >> > Cc: Alexandre Courbot > >> > Cc: stable@vger.kernel.org > >> > Fixes: 10cf4899f8af ("gpiolib: tighten up ACPI legacy gpio looku= ps") > >> > Signed-off-by: Ville Syrj=E4l=E4 > >> > --- > >> > drivers/gpio/gpiolib-acpi.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-= acpi.c > >> > index 682070d20f00..2dc52585e3f2 100644 > >> > --- a/drivers/gpio/gpiolib-acpi.c > >> > +++ b/drivers/gpio/gpiolib-acpi.c > >> > @@ -977,7 +977,7 @@ bool acpi_can_fallback_to_crs(struct acpi_de= vice *adev, const char *con_id) > >> > lookup =3D kmalloc(sizeof(*lookup), GFP_KERNEL); > >> > if (lookup) { > >> > lookup->adev =3D adev; > >> > - lookup->con_id =3D con_id; > >> > + lookup->con_id =3D kstrdup(con_id, GFP_K= ERNEL); > >> > >> Agree with the idea, but where is this newly-allocated memory free= d? > > > > It isn't. Neither is the 'lookup' thing itself. >=20 > Are we ok with that? Leaking memory is slightly better than crashing = a > driver, but that is still not something to be fond of... When would you free it? --=20 Ville Syrj=E4l=E4 Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html