Function dualshock4_get_calibration_data allocates memory to pointer buf .However, the function may exit prematurely due to transfer_failure in this case it does not handle freeing memory.
This patch handles memory deallocation at exit.
Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/691560c4.a70a0220.3124cb.0019.GAE@google.com/T/ Tested-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com Fixes: 947992c7fa9e0 ("HID: playstation: DS4: Fix calibration workaround for clone devices") Cc: stable@vger.kernel.org Signed-off-by: Eslam Khafagy eslam.medhat1993@gmail.com --- v3: * Address issues reported by checkpatch and re-format commit message for better readability * kfree() is safe so no need to check for NULL pointer
v2: https://lore.kernel.org/all/20251116022723.29857-1-eslam.medhat1993@gmail.co... * Adding tag "Cc: stable@vger.kernel.org"
v1: https://lore.kernel.org/all/20251115022323.1395726-1-eslam.medhat1993@gmail....
drivers/hid/hid-playstation.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 128aa6abd10b..05a8522ace4f 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -1994,9 +1994,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) acc_z_plus = get_unaligned_le16(&buf[31]); acc_z_minus = get_unaligned_le16(&buf[33]);
- /* Done parsing the buffer, so let's free it. */ - kfree(buf); - /* * Set gyroscope calibration and normalization parameters. * Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s. @@ -2043,6 +2040,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) ds4->accel_calib_data[2].sens_denom = range_2g;
transfer_failed: + /* First free buf if still allocated */ + kfree(buf); + /* * Sanity check gyro calibration data. This is needed to prevent crashes * during report handling of virtual, clone or broken devices not implementing
On 11/23/25 2:37 AM, Eslam Khafagy wrote:
Function dualshock4_get_calibration_data allocates memory to pointer buf .However, the function may exit prematurely due to transfer_failure in this case it does not handle freeing memory.
This patch handles memory deallocation at exit.
Thanks, Eslam!
Reviewed-by: Max Staudt max@enpas.org
Function dualshock4_get_calibration_data allocates memory to pointer buf .However, the function may exit prematurely due to transfer_failure
transfer failure.?
in this case it does not handle freeing memory.
This patch handles memory deallocation at exit.
Would a corresponding imperative wording become helpful for an improved change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Would a summary phrase like “Prevent memory leak in dualshock4_get_calibration_data()” be nicer?
Regards, Markus
On 22. 11. 25, 18:37, Eslam Khafagy wrote:
Function dualshock4_get_calibration_data allocates memory to pointer buf .However, the function may exit prematurely due to transfer_failure in this case it does not handle freeing memory.
This patch handles memory deallocation at exit.
Isn't this fixed already by: commit 8513c154f8ad7097653dd9bf43d6155e5aad4ab3 Author: Abdun Nihaal nihaal@cse.iitm.ac.in Date: Mon Nov 10 22:45:50 2025 +0530
HID: playstation: Fix memory leak in dualshock4_get_calibration_data() ?
Anyway, this is a typical use-case for __free(). Why not to use that?
Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/691560c4.a70a0220.3124cb.0019.GAE@google.com/T/ Tested-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com Fixes: 947992c7fa9e0 ("HID: playstation: DS4: Fix calibration workaround for clone devices") Cc: stable@vger.kernel.org Signed-off-by: Eslam Khafagy eslam.medhat1993@gmail.com
v3:
- Address issues reported by checkpatch and re-format commit message
for better readability
- kfree() is safe so no need to check for NULL pointer
v2: https://lore.kernel.org/all/20251116022723.29857-1-eslam.medhat1993@gmail.co...
- Adding tag "Cc: stable@vger.kernel.org"
v1: https://lore.kernel.org/all/20251115022323.1395726-1-eslam.medhat1993@gmail....
drivers/hid/hid-playstation.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 128aa6abd10b..05a8522ace4f 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -1994,9 +1994,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) acc_z_plus = get_unaligned_le16(&buf[31]); acc_z_minus = get_unaligned_le16(&buf[33]);
- /* Done parsing the buffer, so let's free it. */
- kfree(buf);
- /*
- Set gyroscope calibration and normalization parameters.
- Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s.
@@ -2043,6 +2040,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) ds4->accel_calib_data[2].sens_denom = range_2g; transfer_failed:
- /* First free buf if still allocated */
- kfree(buf);
- /*
- Sanity check gyro calibration data. This is needed to prevent crashes
- during report handling of virtual, clone or broken devices not implementing
On 11/24/25 3:32 PM, Jiri Slaby wrote:
Isn't this fixed already by: commit 8513c154f8ad7097653dd9bf43d6155e5aad4ab3 Author: Abdun Nihaal nihaal@cse.iitm.ac.in Date: Mon Nov 10 22:45:50 2025 +0530
HID: playstation: Fix memory leak in dualshock4_get_calibration_data() ?
As far as I can see, that patch does indeed fix the same issue, and it is already upstream.
Thanks for the hint - Abdun's patch has been upstreamed quite recently, hence I guess Eslam missed it by accident. But maybe I'm wrong and Eslam can chime in himself?
Anyway, this is a typical use-case for __free(). Why not to use that?
Wow, there's been a lot of interesting stuff happening around cleanup.h. I've been out of the kernel for too long, this looks like fun. Thanks for pointing it out :)
Max
On 11/24/25 16:06, Max Staudt wrote:
On 11/24/25 3:32 PM, Jiri Slaby wrote:
Isn't this fixed already by: commit 8513c154f8ad7097653dd9bf43d6155e5aad4ab3 Author: Abdun Nihaal nihaal@cse.iitm.ac.in Date: Mon Nov 10 22:45:50 2025 +0530
HID: playstation: Fix memory leak in dualshock4_get_calibration_data() ?
As far as I can see, that patch does indeed fix the same issue, and it is already upstream.
Thanks for the hint - Abdun's patch has been upstreamed quite recently, hence I guess Eslam missed it by accident. But maybe I'm wrong and Eslam can chime in himself?
Thank's Max & Jiri, sorry i was sick the past couple of days i missed your replies. yes. that patch fixes it. I guess i missed it because it wasn't merged yet when i submitted v1. So please ignore this patch.
Anyway, this is a typical use-case for __free(). Why not to use that?
Wow, there's been a lot of interesting stuff happening around cleanup.h. I've been out of the kernel for too long, this looks like fun. Thanks for pointing it out :)
Max
Lastly, One question to max, at the beginning of the function dualshock4_get_calibration_data buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); if (!buf) { ret = -ENOMEM; goto transfer_failed; } if the allocation fails. can't we just return here . or do we need to go the the end of the function and do sanity checks at the end?
On 11/26/25 3:19 AM, Eslam Khafagy wrote:
Lastly, One question to max, at the beginning of the function dualshock4_get_calibration_data buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); if (! buf) { ret = -ENOMEM; goto transfer_failed; } if the allocation fails. can't we just return here
Never.
or do we need to go the the end of the function and do sanity checks at the end?
Correct.
Without the sanitisation code, the driver will divide by zero.
Max
linux-stable-mirror@lists.linaro.org
-
Eslam Khafagy -
Jiri Slaby -
Markus Elfring -
Max Staudt