Hi all,
and thanks a lot for your time.
We current git tree I've got this:
[ 10.299221] sony-laptop: Sony Programmable IO Control Driver v0.6.
[ 10.299237] sony-laptop: detected Type3 model
[ 10.300885] input: Sony Vaio Keys as
/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:45/SNY6001:00/input/input3
[ 10.301516] input: Sony Vaio Jogdial as /devices/virtual/input/input4
[ 10.302366] sony-laptop: device allocated minor is 58
[ 10.310838] sony-laptop: Sony Notebook Control Driver v0.6.
[ 10.310940] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 10.310996] IP: [<f816c0b0>] sony_find_snc_handle+0x10/0x70 [sony_laptop]
[ 10.311023] *pde = 00000000
[ 10.311023] Oops: 0000 [#1] PREEMPT SMP
[ 10.311083] last sysfs file: /sys/bus/platform/drivers/sony-laptop/uevent
[ 10.311083] Modules linked in: sony_laptop(+) psmouse loop
processor snd_hda_codec rfkill evdev i915 fbcon font bitblit
softcursor drm_kms_helper intel_agp cfbcopyarea video sky2 intel_gtt
thermal thermal_sys button uhci_hcd cfbimgblt cfbfillrect
[ 10.311083] Pid: 1793, comm: modprobe Not tainted 2.6.39-rc1-test+
#1 Sony Corporation VGN-SZ1VP/VAIO
[ 10.311083] EIP: 0060:[<f816c0b0>] EFLAGS: 00010296 CPU: 0
[ 10.311083] EIP is at sony_find_snc_handle+0x10/0x70 [sony_laptop]
[ 10.311083] EAX: 0000012f EBX: 00000000 ECX: 00000000 EDX: 00000286
[ 10.311083] ESI: f47d9e78 EDI: f500b400 EBP: 00000000 ESP: f47d9e18
[ 10.311083] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 10.311083] Process modprobe (pid: 1793, ti=f47d8000 task=f4bc0e40
task.ti=f47d8000)
[ 10.311083] Stack:
[ 10.311083] 00000000 00000000 00000000 00000000 f816e694 f817126d
f8171250 f817124c
[ 10.311083] f4943460 0000447f f47d9e74 f5083bd0 f4935b10 00000000
c110ef27 c110decf
[ 10.311083] f47d9e90 f4935ae0 f47d9e90 c110e873 00000000 41ed9559
f44c34e0 f5083bd0
[ 10.311083] Call Trace:
[ 10.311083] [<f816e694>] ? sony_nc_add+0x2a4/0x930 [sony_laptop]
[ 10.311083] [<c110ef27>] ? sysfs_do_create_link+0xb7/0x1d0
[ 10.311083] [<c110decf>] ? __sysfs_add_one+0x1f/0xf0
[ 10.311083] [<c110e873>] ? sysfs_addrm_finish+0x13/0xc0
[ 10.311083] [<c11e9862>] ? acpi_device_probe+0x37/0xec
[ 10.311083] [<c1247c97>] ? driver_probe_device+0x77/0x190
[ 10.311083] [<c11e9c58>] ? acpi_match_device_ids+0x27/0x49
[ 10.311083] [<c1247e29>] ? __driver_attach+0x79/0x80
[ 10.311083] [<c1247db0>] ? driver_probe_device+0x190/0x190
[ 10.311083] [<c1247522>] ? bus_for_each_dev+0x52/0x80
[ 10.311083] [<c1247b26>] ? driver_attach+0x16/0x20
[ 10.311083] [<c1247db0>] ? driver_probe_device+0x190/0x190
[ 10.311083] [<c1246f07>] ? bus_add_driver+0x177/0x240
[ 10.311083] [<c11e979f>] ? acpi_device_hid+0x14/0x14
[ 10.311083] [<c12483b3>] ? driver_register+0x63/0x120
[ 10.311083] [<f827104d>] ? sony_laptop_init+0x4d/0x1000 [sony_laptop]
[ 10.311083] [<c1001123>] ? do_one_initcall+0x33/0x170
[ 10.311083] [<f8271000>] ? 0xf8270fff
[ 10.311083] [<c1065b49>] ? sys_init_module+0xe9/0x350
[ 10.311083] [<c10c1b51>] ? sys_write+0x41/0x80
[ 10.311083] [<c137a110>] ? sysenter_do_call+0x12/0x26
[ 10.311083] Code: 2c 17 f8 5b 3b 15 28 2c 17 f8 ba 41 00 00 00 0f
45 c2 c3 90 8d b4 26 00 00 00 00 53 31 db 83 ec 0c 8b 0d dc 2c 17 f8
8d 74 26 00 <0f> b7 14 59 39 c2 74 20 43 83 fb 10 75 f2 8b 15 a8 2c 17
f8 85
[ 10.311083] EIP: [<f816c0b0>] sony_find_snc_handle+0x10/0x70
[sony_laptop] SS:ESP 0068:f47d9e18
[ 10.311083] CR2: 0000000000000000
[ 10.354377] ---[ end trace 1c42dff203c3e1bc ]---
Git bisect blames this commit:
commit 9af0e0fb70ed8e2387323c19496a7e174363f7b6
Author: Anssi Hannula <[email protected]>
Date: Sun Feb 20 20:07:20 2011 +0200
hp-wmi: check query return value in hp_wmi_perform_query
Check BIOS provided return value code in hp_wmi_perform_query and print
a warning on error. Printing is suppressed for HPWMI_RET_UNKNOWN_CMDTYPE
which is returned when the command type is unsupported.
Signed-off-by: Anssi Hannula <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
drivers/platform/x86/hp-wmi.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
In attachment other info.
If you need more details, please tell me.
Thanks a lot for your time,
Andrea
On Fri, Apr 01, 2011 at 07:16:43PM +0200, Andrea Gelmini wrote:
> Git bisect blames this commit:
Pretty convinced that git bisect's got that wrong, given that it doesn't
touch any Sony code. I'll take a look at the Sony patches, thanks for
the report!
--
Matthew Garrett | [email protected]
On 01.04.2011 20:22, Matthew Garrett wrote:
> On Fri, Apr 01, 2011 at 07:16:43PM +0200, Andrea Gelmini wrote:
>
>> Git bisect blames this commit:
>
> Pretty convinced that git bisect's got that wrong, given that it doesn't
> touch any Sony code. I'll take a look at the Sony patches, thanks for
> the report!
The same issue was posted yesterday on platform-driver-x86@ as "NULL
pointer dereference in sony-laptop" by Alessandro Guido, with a
preliminary patch from Mattia Dongili:
http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/1720
--
Anssi Hannula
2011/4/1 Matthew Garrett <[email protected]>:
> touch any Sony code. I'll take a look at the Sony patches, thanks for
> the report!
Hi Matthew,
and thanks a lot for your quick answer.
Maybe I messed up with bisect trying to find the origin of two regression.
One was this. The other one is this one.
Hibernation (to disk, of course) doesn't work no more.
Screen blanks and nothing happens. No HD activity, and so on. I also
waited some minutes.
Well, bisect blame this commit:
commit 2a4f0c81adcd1f812a63bc9106be2fd26f437730
Author: Mattia Dongili <[email protected]>
Date: Sat Feb 19 11:52:30 2011 +0900
sony-laptop: cache handles and report them via sysfs
Avoid calling into acpi each time we need to lookup a method handle
and report the available handles to ease collection of information when
debugging issues. Also move initialization of the platform driver
earlier to allow adding files from other setup functions.
Signed-off-by: Mattia Dongili <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
drivers/platform/x86/sony-laptop.c | 97 ++++++++++++++++++++++++++++++------
1 files changed, 81 insertions(+), 16 deletions(-)
Do you have any suggestion to better track down this issue?
It's not possibile to simply revert it.
Thanks a lot for your time,
Andrea
2011/4/1 Anssi Hannula <[email protected]>:
> The same issue was posted yesterday on platform-driver-x86@ as "NULL
> pointer dereference in sony-laptop" by Alessandro Guido, with a
> preliminary patch from Mattia Dongili:
Hi Anssi,
and thanks a lot for your quick answer.
Ciao,
Andrea
On Sat, Apr 02, 2011 at 11:44:57AM +0200, Andrea Gelmini wrote:
> 2011/4/1 Matthew Garrett <[email protected]>:
> > touch any Sony code. I'll take a look at the Sony patches, thanks for
> > the report!
>
> Hi Matthew,
> and thanks a lot for your quick answer.
> Maybe I messed up with bisect trying to find the origin of two regression.
> One was this. The other one is this one.
>
> Hibernation (to disk, of course) doesn't work no more.
> Screen blanks and nothing happens. No HD activity, and so on. I also
> waited some minutes.
>
> Well, bisect blame this commit:
>
> commit 2a4f0c81adcd1f812a63bc9106be2fd26f437730
> Author: Mattia Dongili <[email protected]>
> Date: Sat Feb 19 11:52:30 2011 +0900
>
> sony-laptop: cache handles and report them via sysfs
>
> Avoid calling into acpi each time we need to lookup a method handle
> and report the available handles to ease collection of information when
> debugging issues. Also move initialization of the platform driver
> earlier to allow adding files from other setup functions.
>
> Signed-off-by: Mattia Dongili <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> drivers/platform/x86/sony-laptop.c | 97 ++++++++++++++++++++++++++++++------
> 1 files changed, 81 insertions(+), 16 deletions(-)
>
> Do you have any suggestion to better track down this issue?
> It's not possibile to simply revert it.
As already reported the following patch should fix the issue:
commit 88d25cbfda526567dabf056a868d8ff5f22a962e
Author: Mattia Dongili <[email protected]>
Date: Fri Apr 1 10:01:41 2011 +0900
sony-laptop: fix early NULL pointer dereference
The SNC acpi driver could get early notifications before it fully
initializes and that could lead to dereferencing the sony_nc_handles
structure pointer that is still NULL at that stage.
Make sure we return early from the handle lookup function in these
cases.
Signed-off-by: Mattia Dongili <[email protected]>
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index b2ce172..7082c55 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct platform_device *pd)
static int sony_find_snc_handle(int handle)
{
int i;
+
+ /* not initialized yet, return early */
+ if (!handles)
+ return -1;
+
for (i = 0; i < 0x10; i++) {
if (handles->cap[i] == handle) {
dprintk("found handle 0x%.4x (offset: 0x%.2x)\n",
--
mattia
:wq!
On 4/2/11, Mattia Dongili <[email protected]> wrote:
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct
> platform_device *pd)
> static int sony_find_snc_handle(int handle)
> {
> int i;
> +
> + /* not initialized yet, return early */
> + if (!handles)
> + return -1;
-1 is -EPERM. That's not the right error code here. Maybe -EINVAL?
regards,
dan carpenter
On Sat, Apr 02, 2011 at 02:55:38PM +0300, Dan Carpenter wrote:
> On 4/2/11, Mattia Dongili <[email protected]> wrote:
> > --- a/drivers/platform/x86/sony-laptop.c
> > +++ b/drivers/platform/x86/sony-laptop.c
> > @@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct
> > platform_device *pd)
> > static int sony_find_snc_handle(int handle)
> > {
> > int i;
> > +
> > + /* not initialized yet, return early */
> > + if (!handles)
> > + return -1;
>
> -1 is -EPERM. That's not the right error code here. Maybe -EINVAL?
this error is not propagated to userspace. If necessary I can review all
error codes in the sony-laptop internal functions (where -1 is a fairly
common return code for error conditions).
I remember a discussion on LKML recently about error codes but I'm not
sure what the outcome was (if any).
--
mattia
:wq!
On 4/2/11, Mattia Dongili <[email protected]> wrote:
> On Sat, Apr 02, 2011 at 02:55:38PM +0300, Dan Carpenter wrote:
>> On 4/2/11, Mattia Dongili <[email protected]> wrote:
>> > --- a/drivers/platform/x86/sony-laptop.c
>> > +++ b/drivers/platform/x86/sony-laptop.c
>> > @@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct
>> > platform_device *pd)
>> > static int sony_find_snc_handle(int handle)
>> > {
>> > int i;
>> > +
>> > + /* not initialized yet, return early */
>> > + if (!handles)
>> > + return -1;
>>
>> -1 is -EPERM. That's not the right error code here. Maybe -EINVAL?
>
> this error is not propagated to userspace. If necessary I can review all
> error codes in the sony-laptop internal functions (where -1 is a fairly
> common return code for error conditions).
That would be good, but it's going beyond the call of duty. The main thing
is to not introduce new slop.
>
> I remember a discussion on LKML recently about error codes but I'm not
> sure what the outcome was (if any).
>
People sometimes think -1 is a generic error code, but it's not. It has
a specific wrong meaning. In fact, -1 is never the right error code.
It's should
either be -EPERM, or the appropriate error code.
regards,
dan carpenter
Author: Mattia Dongili <[email protected]>
Date: Fri Apr 1 10:01:41 2011 +0900
sony-laptop: fix early NULL pointer dereference
The SNC acpi driver could get early notifications before it fully
initializes and that could lead to dereferencing the sony_nc_handles
structure pointer that is still NULL at that stage.
Make sure we return early from the handle lookup function in these
cases.
Signed-off-by: Mattia Dongili <[email protected]>
---
Hi Matthew,
if it's not too late, can you pick this one up instead of the previous
one (89ec2feafaedd759e53346d641f60863a14cfb9e)?
If it's too late I'll try and do a round of return value fixes later.
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index b2ce172..de79c18 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct platform_device *pd)
static int sony_find_snc_handle(int handle)
{
int i;
+
+ /* not initialized yet, return early */
+ if (!handles)
+ return -EINVAL;
+
for (i = 0; i < 0x10; i++) {
if (handles->cap[i] == handle) {
dprintk("found handle 0x%.4x (offset: 0x%.2x)\n",
--
mattia
:wq!
On Mon, Apr 4, 2011 at 8:44 PM, Mattia Dongili <[email protected]> wrote:
> Author: Mattia Dongili <[email protected]>
> Date: Fri Apr 1 10:01:41 2011 +0900
>
I think most of the patches doesn't include these above lines nor the
subject line in the description. Also when submitting a patch to the
list, please don't forget to put [PATCH] on the subject line.
git format-patch should output the right format.
> sony-laptop: fix early NULL pointer dereference
>
> The SNC acpi driver could get early notifications before it fully
> initializes and that could lead to dereferencing the sony_nc_handles
> structure pointer that is still NULL at that stage.
> Make sure we return early from the handle lookup function in these
> cases.
>
> Signed-off-by: Mattia Dongili <[email protected]>
> ---
>
> Hi Matthew,
> if it's not too late, can you pick this one up instead of the previous
> one (89ec2feafaedd759e53346d641f60863a14cfb9e)?
> If it's too late I'll try and do a round of return value fixes later.
>
> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index b2ce172..de79c18 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct platform_device *pd)
> static int sony_find_snc_handle(int handle)
> {
> int i;
> +
> + /* not initialized yet, return early */
This comment is useless, it is just repeating what the codes does ;) I
think you can just remove it.
> + if (!handles)
> + return -EINVAL;
> +
> for (i = 0; i < 0x10; i++) {
> if (handles->cap[i] == handle) {
> dprintk("found handle 0x%.4x (offset: 0x%.2x)\n",
> --
> mattia
> :wq!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On 4/5/11, Thiago Farina <[email protected]> wrote:
> On Mon, Apr 4, 2011 at 8:44 PM, Mattia Dongili <[email protected]> wrote:
>> Author: Mattia Dongili <[email protected]>
>> Date: Fri Apr 1 10:01:41 2011 +0900
>>
> I think most of the patches doesn't include these above lines nor the
> subject line in the description.
Just ignore those lines. It's a git thing.
> Also when submitting a patch to the
> list, please don't forget to put [PATCH] on the subject line.
>
Yeah. [patch v2]. But it's too late to fix that so don't worry about it.
> git format-patch should output the right format.
>
>> sony-laptop: fix early NULL pointer dereference
>>
>> The SNC acpi driver could get early notifications before it fully
>> initializes and that could lead to dereferencing the sony_nc_handles
>> structure pointer that is still NULL at that stage.
>> Make sure we return early from the handle lookup function in these
>> cases.
>>
>> Signed-off-by: Mattia Dongili <[email protected]>
>> ---
>>
>> Hi Matthew,
>> if it's not too late, can you pick this one up instead of the previous
>> one (89ec2feafaedd759e53346d641f60863a14cfb9e)?
>> If it's too late I'll try and do a round of return value fixes later.
Don't worry about it. Probably the real fix is to make checkpatch.pl
complain if you return -1 instead of a proper error code.
>>
>> diff --git a/drivers/platform/x86/sony-laptop.c
>> b/drivers/platform/x86/sony-laptop.c
>> index b2ce172..de79c18 100644
>> --- a/drivers/platform/x86/sony-laptop.c
>> +++ b/drivers/platform/x86/sony-laptop.c
>> @@ -810,6 +810,11 @@ static int sony_nc_handles_cleanup(struct
>> platform_device *pd)
>> static int sony_find_snc_handle(int handle)
>> {
>> int i;
>> +
>> + /* not initialized yet, return early */
> This comment is useless, it is just repeating what the codes does ;) I
> think you can just remove it.
Whatever... Let's just merge this fix and let's move on.
regards,
dan carpenter
>
>> + if (!handles)
>> + return -EINVAL;
>> +
>> for (i = 0; i < 0x10; i++) {
>> if (handles->cap[i] == handle) {
>> dprintk("found handle 0x%.4x (offset: 0x%.2x)\n",