From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>
Cc: Elena Reshetova <elena.reshetova@intel.com>,
David Windsor <dwindsor@gmail.com>, Kees Cook <kees@kernel.org>,
Hans Liljestrand <ishkamiel@gmail.com>,
linux-mips@vger.kernel.org, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 4/4] Revert "drivers: convert sbd_duart.map_guard from atomic_t to refcount_t"
Date: Mon, 13 Apr 2026 04:28:53 +0100 (BST) [thread overview]
Message-ID: <alpine.DEB.2.21.2604130416440.29980@angie.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.2.21.2604130239560.29980@angie.orcam.me.uk>
Revert commit 22a33651a56f ("drivers: convert sbd_duart.map_guard from
atomic_t to refcount_t"), which broke perfectly valid code:
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at lib/refcount.c:114 sbd_request_port+0x54/0x140
refcount_t: increment on 0; use-after-free.
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc2+ #34
Stack : 0000000014001fe0 0000000000000000 ffffffff80830000 0000000000000000
ffffffff8127bc7a ffffffff8016fe08 ffffffff808d0000 ffffffff808d0000
ffffffff807aa828 ffffffff80822337 ffffffff808ce188 a8000001860b0000
0000000000000001 0000000000000001 00000000000001c8 ffffffff808a3090
00000000000000bb ffffffff801b09d4 a80000018609bb68 ffffffff801231cc
ffffffff812a0000 ffffffff80171388 0000000000001000 ffffffff807aa828
0000000000000001 0000000000000001 0000000000000000 0000000000000000
0000000000000000 a80000018609bab0 0000000000000000 ffffffff803c47cc
0000000000000000 0000000000000000 0000000000000000 0000000000000000
ffffffff807cb648 ffffffff8010bff8 0000000014001fe1 ffffffff803c47cc
...
Call Trace:
[<ffffffff8010bff8>] show_stack+0x28/0x88
[<ffffffff803c47cc>] dump_stack+0x8c/0xc0
[<ffffffff801aff5c>] __warn+0xe0/0x114
[<ffffffff801233f0>] warn_slowpath_fmt+0x40/0x50
[<ffffffff80455bcc>] sbd_request_port+0x54/0x140
[<ffffffff804563a4>] sbd_config_port+0x2c/0x68
---[ end trace f666d696412caa3e ]---
(report at the offending commit) -- sbd_request_port() is called twice
per DUART instance, to reserve a resource holding the control register
block shared between the two channels, so there's no slightest chance
for an overflow. Also this doesn't stop the driver from working and
it's just the reservation that is missing as a result, i.e.:
10060100-100601ff : sb1250-duart
10060200-100602ff : sb1250-duart
as from the offending change, vs:
10060100-100601ff : sb1250-duart
10060200-100602ff : sb1250-duart
10060300-100603ff : sb1250-duart
beforehand, which is surely why the breakage has gone so long unnoticed.
"If it ain't broke, don't fix it," so just revert the broken commit.
Fixes: 22a33651a56f ("drivers: convert sbd_duart.map_guard from atomic_t to refcount_t")
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
drivers/tty/serial/sb1250-duart.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
linux-serial-sb1250-duart-map-guard-atomic.diff
Index: linux-macro/drivers/tty/serial/sb1250-duart.c
===================================================================
--- linux-macro.orig/drivers/tty/serial/sb1250-duart.c
+++ linux-macro/drivers/tty/serial/sb1250-duart.c
@@ -34,8 +34,8 @@
#include <linux/tty_flip.h>
#include <linux/types.h>
-#include <linux/refcount.h>
-#include <linux/io.h>
+#include <linux/atomic.h>
+#include <asm/io.h>
#include <asm/sibyte/sb1250.h>
#include <asm/sibyte/sb1250_uart.h>
@@ -86,7 +86,7 @@ struct sbd_port {
struct sbd_duart {
struct sbd_port sport[2];
unsigned long mapctrl;
- refcount_t map_guard;
+ atomic_t map_guard;
};
#define to_sport(uport) container_of(uport, struct sbd_port, port)
@@ -662,13 +662,15 @@ static void sbd_release_port(struct uart
{
struct sbd_port *sport = to_sport(uport);
struct sbd_duart *duart = sport->duart;
+ int map_guard;
iounmap(sport->memctrl);
sport->memctrl = NULL;
iounmap(uport->membase);
uport->membase = NULL;
- if(refcount_dec_and_test(&duart->map_guard))
+ map_guard = atomic_add_return(-1, &duart->map_guard);
+ if (!map_guard)
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
}
@@ -704,6 +706,7 @@ static int sbd_request_port(struct uart_
{
const char *err = KERN_ERR "sbd: Unable to reserve MMIO resource\n";
struct sbd_duart *duart = to_sport(uport)->duart;
+ int map_guard;
int ret = 0;
if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
@@ -711,11 +714,11 @@ static int sbd_request_port(struct uart_
printk(err);
return -EBUSY;
}
- refcount_inc(&duart->map_guard);
- if (refcount_read(&duart->map_guard) == 1) {
+ map_guard = atomic_add_return(1, &duart->map_guard);
+ if (map_guard == 1) {
if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
"sb1250-duart")) {
- refcount_dec(&duart->map_guard);
+ atomic_add(-1, &duart->map_guard);
printk(err);
ret = -EBUSY;
}
@@ -723,7 +726,8 @@ static int sbd_request_port(struct uart_
if (!ret) {
ret = sbd_map_port(uport);
if (ret) {
- if (refcount_dec_and_test(&duart->map_guard))
+ map_guard = atomic_add_return(-1, &duart->map_guard);
+ if (!map_guard)
release_mem_region(duart->mapctrl,
DUART_CHANREG_SPACING);
}
next prev parent reply other threads:[~2026-04-13 3:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-13 3:28 [PATCH 0/4] MIPS: SiByte: Fix serial device regressions Maciej W. Rozycki
2026-04-13 3:28 ` [PATCH 1/4] MIPS: SiByte: Fix console message clobbering at channel resets Maciej W. Rozycki
2026-04-13 3:28 ` [PATCH 2/4] MIPS: SiByte: Fix bootconsole handover lockup Maciej W. Rozycki
2026-04-13 3:28 ` [PATCH 3/4] MIPS: SiByte: Convert to use a platform device Maciej W. Rozycki
2026-04-13 3:28 ` Maciej W. Rozycki [this message]
2026-04-26 20:45 ` [PATCH 4/4] Revert "drivers: convert sbd_duart.map_guard from atomic_t to refcount_t" Greg Kroah-Hartman
2026-04-27 14:13 ` Maciej W. Rozycki
2026-04-27 16:31 ` Greg Kroah-Hartman
2026-04-27 20:06 ` Maciej W. Rozycki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2604130416440.29980@angie.orcam.me.uk \
--to=macro@orcam.me.uk \
--cc=dwindsor@gmail.com \
--cc=elena.reshetova@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=ishkamiel@gmail.com \
--cc=jirislaby@kernel.org \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=tsbogend@alpha.franken.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox