[PATCH] serdev: ttyport: Clear serport->tty after freeing

[PATCH] serdev: ttyport: Clear serport->tty after freeing

[Hans de Goede]

From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>

Both error paths in ttyport_open(), and ttyport_close(), release the tty
with tty_release_struct() and leave serport->tty pointing at freed memory.

The serdev core itself never dereferences it afterwards. However, a buggy
driver could easily trigger a use-after-free by calling a ttyport operation
on a port that is not open, or by calling close() twice. While those
drivers should be fixed, clearing the pointer, makes them fail
deterministically instead of touching freed memory.

Note that a driver which currently double closes gets away with it only by
chance, depending on whether the freed tty has been reused. After this
change such a driver oopses immediately instead, which is the intended
outcome, but it may surface latent bugs elsewhere.

Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/tty/serdev/serdev-ttyport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index bab1b143b8a6..b6638f9f40d2 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -137,6 +137,7 @@ static int ttyport_open(struct serdev_controller *ctrl)
 err_unlock:
 	tty_unlock(tty);
 	tty_release_struct(tty, serport->tty_idx);
+	serport->tty = NULL;
 
 	return ret;
 }
@@ -154,6 +155,7 @@ static void ttyport_close(struct serdev_controller *ctrl)
 	tty_unlock(tty);
 
 	tty_release_struct(tty, serport->tty_idx);
+	serport->tty = NULL;
 }
 
 static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigned int speed)
-- 
2.55.0

RE: serdev: ttyport: Clear serport->tty after freeing

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1148478

---Test result---

Test Summary:
CheckPatch                    PASS      0.52 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.10 seconds
GitLint                       PASS      0.25 seconds
SubjectPrefix                 FAIL      0.10 seconds
BuildKernel                   PASS      18.72 seconds
CheckAllWarning               PASS      21.80 seconds
CheckSparse                   PASS      19.98 seconds
BuildKernel32                 PASS      17.30 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      311.28 seconds
TestRunner_l2cap-tester       PASS      52.41 seconds
TestRunner_iso-tester         PASS      60.32 seconds
TestRunner_bnep-tester        PASS      13.37 seconds
TestRunner_mgmt-tester        FAIL      184.01 seconds
TestRunner_rfcomm-tester      PASS      18.32 seconds
TestRunner_sco-tester         PASS      24.48 seconds
TestRunner_ioctl-tester       PASS      20.20 seconds
TestRunner_mesh-tester        FAIL      19.39 seconds
TestRunner_smp-tester         PASS      18.21 seconds
TestRunner_userchan-tester    PASS      14.49 seconds
TestRunner_6lowpan-tester     PASS      16.63 seconds
IncrementalBuild              PASS      18.44 seconds

Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.195 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.937 seconds
Mesh - Send cancel - 2                               Timed out    1.990 seconds


https://github.com/bluez/bluetooth-next/pull/617

---
Regards,
Linux Bluetooth

Re: [PATCH] serdev: ttyport: Clear serport->tty after freeing

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

On Wed, 2026-08-19 at 14:57 +0200, Hans de Goede wrote:
> From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
> 
> Both error paths in ttyport_open(), and ttyport_close(), release the tty
> with tty_release_struct() and leave serport->tty pointing at freed memory.
> 
> The serdev core itself never dereferences it afterwards. However, a buggy
> driver could easily trigger a use-after-free by calling a ttyport operation
> on a port that is not open, or by calling close() twice. While those
> drivers should be fixed, clearing the pointer, makes them fail
> deterministically instead of touching freed memory.
> 
> Note that a driver which currently double closes gets away with it only by
> chance, depending on whether the freed tty has been reused. After this
> change such a driver oopses immediately instead, which is the intended
> outcome, but it may surface latent bugs elsewhere.
> 
> Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

I recently took the role as maintainer for serdev [1].

Please add me to CC if sending another revision.
I only look loosly at the linux-serial mailing list, so I might
otherwise miss a patch.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/MAINTAINERS?h=driver-core-next&id=6b71fef6f05cee64ebbaa8aec6dc688169f47e9c

> ---
>  drivers/tty/serdev/serdev-ttyport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index bab1b143b8a6..b6638f9f40d2 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -137,6 +137,7 @@ static int ttyport_open(struct serdev_controller *ctrl)
>  err_unlock:
>  	tty_unlock(tty);
>  	tty_release_struct(tty, serport->tty_idx);
> +	serport->tty = NULL;
>  
>  	return ret;
>  }
> @@ -154,6 +155,7 @@ static void ttyport_close(struct serdev_controller *ctrl)
>  	tty_unlock(tty);
>  
>  	tty_release_struct(tty, serport->tty_idx);
> +	serport->tty = NULL;
>  }
>  
>  static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigned int speed)

Always good to see hardening.

Reviewed-by: Markus Probst <markus.probst@posteo.de>

Thanks
- Markus Probst


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]