Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields
@ 2019-12-17 14:02 Greg Kroah-Hartman
  2019-12-17 15:47 ` Dmitry Safonov
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-17 14:02 UTC (permalink / raw)
  To: Kukjin Kim, Hyunki Koo, HYUN-KI KOO, Shinbeom Choi,
	Krzysztof Kozlowski, Dmitry Safonov
  Cc: Jiri Slaby, linux-serial, linux-kernel

The samsung_tty driver was trying to abuse the struct uart_port by using
two "empty" bytes for its own use.  That's not ok, and was found by
removing those fields from the structure.

Move the variables into the port-specific structure, which is where
everything else for this port already is.  There is no space wasted here
as there was an empty "hole" in the structure already for these bytes.

Cc: Kukjin Kim <kgene@kernel.org>
Cc: Hyunki Koo <kkoos00@naver.com>
Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
Cc: Shinbeom Choi <sbeom.choi@samsung.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/samsung_tty.c | 46 ++++++++++++++++----------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 2fd4f2de7083..5b5f249cbf1f 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -56,10 +56,6 @@
 #define S3C24XX_TX_DMA			2
 #define S3C24XX_RX_PIO			1
 #define S3C24XX_RX_DMA			2
-/* macros to change one thing to another */
-
-#define tx_enabled(port) ((port)->unused[0])
-#define rx_enabled(port) ((port)->unused[1])
 
 /* flag to ignore all characters coming in */
 #define RXSTAT_DUMMY_READ (0x10000000)
@@ -123,6 +119,8 @@ struct s3c24xx_uart_dma {
 struct s3c24xx_uart_port {
 	unsigned char			rx_claimed;
 	unsigned char			tx_claimed;
+	unsigned char			rx_enabled;
+	unsigned char			tx_enabled;
 	unsigned int			pm_level;
 	unsigned long			baudclk_rate;
 	unsigned int			min_dma_size;
@@ -223,6 +221,7 @@ static int s3c24xx_serial_has_interrupt_mask(struct uart_port *port)
 
 static void s3c24xx_serial_rx_enable(struct uart_port *port)
 {
+	struct s3c24xx_uart_port *ourport = to_ourport(port);
 	unsigned long flags;
 	unsigned int ucon, ufcon;
 	int count = 10000;
@@ -240,12 +239,13 @@ static void s3c24xx_serial_rx_enable(struct uart_port *port)
 	ucon |= S3C2410_UCON_RXIRQMODE;
 	wr_regl(port, S3C2410_UCON, ucon);
 
-	rx_enabled(port) = 1;
+	ourport->rx_enabled = 1;
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void s3c24xx_serial_rx_disable(struct uart_port *port)
 {
+	struct s3c24xx_uart_port *ourport = to_ourport(port);
 	unsigned long flags;
 	unsigned int ucon;
 
@@ -255,7 +255,7 @@ static void s3c24xx_serial_rx_disable(struct uart_port *port)
 	ucon &= ~S3C2410_UCON_RXIRQMODE;
 	wr_regl(port, S3C2410_UCON, ucon);
 
-	rx_enabled(port) = 0;
+	ourport->rx_enabled = 0;
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
@@ -267,7 +267,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
 	struct dma_tx_state state;
 	int count;
 
-	if (!tx_enabled(port))
+	if (!ourport->tx_enabled)
 		return;
 
 	if (s3c24xx_serial_has_interrupt_mask(port))
@@ -287,7 +287,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port)
 		port->icount.tx += count;
 	}
 
-	tx_enabled(port) = 0;
+	ourport->tx_enabled = 0;
 	ourport->tx_in_progress = 0;
 
 	if (port->flags & UPF_CONS_FLOW)
@@ -445,11 +445,11 @@ static void s3c24xx_serial_start_tx(struct uart_port *port)
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
 	struct circ_buf *xmit = &port->state->xmit;
 
-	if (!tx_enabled(port)) {
+	if (!ourport->tx_enabled) {
 		if (port->flags & UPF_CONS_FLOW)
 			s3c24xx_serial_rx_disable(port);
 
-		tx_enabled(port) = 1;
+		ourport->tx_enabled = 1;
 		if (!ourport->dma || !ourport->dma->tx_chan)
 			s3c24xx_serial_start_tx_pio(ourport);
 	}
@@ -494,14 +494,14 @@ static void s3c24xx_serial_stop_rx(struct uart_port *port)
 	enum dma_status dma_status;
 	unsigned int received;
 
-	if (rx_enabled(port)) {
+	if (ourport->rx_enabled) {
 		dev_dbg(port->dev, "stopping rx\n");
 		if (s3c24xx_serial_has_interrupt_mask(port))
 			s3c24xx_set_bit(port, S3C64XX_UINTM_RXD,
 					S3C64XX_UINTM);
 		else
 			disable_irq_nosync(ourport->rx_irq);
-		rx_enabled(port) = 0;
+		ourport->rx_enabled = 0;
 	}
 	if (dma && dma->rx_chan) {
 		dmaengine_pause(dma->tx_chan);
@@ -723,9 +723,9 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 		if (port->flags & UPF_CONS_FLOW) {
 			int txe = s3c24xx_serial_txempty_nofifo(port);
 
-			if (rx_enabled(port)) {
+			if (ourport->rx_enabled) {
 				if (!txe) {
-					rx_enabled(port) = 0;
+					ourport->rx_enabled = 0;
 					continue;
 				}
 			} else {
@@ -733,7 +733,7 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 					ufcon = rd_regl(port, S3C2410_UFCON);
 					ufcon |= S3C2410_UFCON_RESETRX;
 					wr_regl(port, S3C2410_UFCON, ufcon);
-					rx_enabled(port) = 1;
+					ourport->rx_enabled = 1;
 					return;
 				}
 				continue;
@@ -1084,7 +1084,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
 	if (ourport->tx_claimed) {
 		if (!s3c24xx_serial_has_interrupt_mask(port))
 			free_irq(ourport->tx_irq, ourport);
-		tx_enabled(port) = 0;
+		ourport->tx_enabled = 0;
 		ourport->tx_claimed = 0;
 		ourport->tx_mode = 0;
 	}
@@ -1093,7 +1093,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
 		if (!s3c24xx_serial_has_interrupt_mask(port))
 			free_irq(ourport->rx_irq, ourport);
 		ourport->rx_claimed = 0;
-		rx_enabled(port) = 0;
+		ourport->rx_enabled = 0;
 	}
 
 	/* Clear pending interrupts and mask all interrupts */
@@ -1115,7 +1115,7 @@ static int s3c24xx_serial_startup(struct uart_port *port)
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
 	int ret;
 
-	rx_enabled(port) = 1;
+	ourport->rx_enabled = 1;
 
 	ret = request_irq(ourport->rx_irq, s3c24xx_serial_rx_chars, 0,
 			  s3c24xx_serial_portname(port), ourport);
@@ -1129,7 +1129,7 @@ static int s3c24xx_serial_startup(struct uart_port *port)
 
 	dev_dbg(port->dev, "requesting tx irq...\n");
 
-	tx_enabled(port) = 1;
+	ourport->tx_enabled = 1;
 
 	ret = request_irq(ourport->tx_irq, s3c24xx_serial_tx_chars, 0,
 			  s3c24xx_serial_portname(port), ourport);
@@ -1176,9 +1176,9 @@ static int s3c64xx_serial_startup(struct uart_port *port)
 	}
 
 	/* For compatibility with s3c24xx Soc's */
-	rx_enabled(port) = 1;
+	ourport->rx_enabled = 1;
 	ourport->rx_claimed = 1;
-	tx_enabled(port) = 0;
+	ourport->tx_enabled = 0;
 	ourport->tx_claimed = 1;
 
 	spin_lock_irqsave(&port->lock, flags);
@@ -2112,9 +2112,9 @@ static int s3c24xx_serial_resume_noirq(struct device *dev)
 		if (s3c24xx_serial_has_interrupt_mask(port)) {
 			unsigned int uintm = 0xf;
 
-			if (tx_enabled(port))
+			if (ourport->tx_enabled)
 				uintm &= ~S3C64XX_UINTM_TXD_MSK;
-			if (rx_enabled(port))
+			if (ourport->rx_enabled)
 				uintm &= ~S3C64XX_UINTM_RXD_MSK;
 			clk_prepare_enable(ourport->clk);
 			if (!IS_ERR(ourport->baudclk))
-- 
2.24.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields
  2019-12-17 14:02 [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields Greg Kroah-Hartman
@ 2019-12-17 15:47 ` Dmitry Safonov
  2019-12-17 15:52   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Safonov @ 2019-12-17 15:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Kukjin Kim, Hyunki Koo, HYUN-KI KOO,
	Shinbeom Choi, Krzysztof Kozlowski
  Cc: Jiri Slaby, linux-serial, linux-kernel

On 12/17/19 2:02 PM, Greg Kroah-Hartman wrote:
> The samsung_tty driver was trying to abuse the struct uart_port by using
> two "empty" bytes for its own use.  That's not ok, and was found by
> removing those fields from the structure.
> 
> Move the variables into the port-specific structure, which is where
> everything else for this port already is.  There is no space wasted here
> as there was an empty "hole" in the structure already for these bytes.

Thanks!
Sorry for not noticing this myself.

> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Hyunki Koo <kkoos00@naver.com>
> Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
> Cc: Shinbeom Choi <sbeom.choi@samsung.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Dmitry Safonov <dima@arista.com>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I see you already applied it to your tree, but in case it helps anything:
Reviewed-by: Dmitry Safonov <dima@arista.com>

Thanks,
          Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields
  2019-12-17 15:47 ` Dmitry Safonov
@ 2019-12-17 15:52   ` Greg Kroah-Hartman
  2019-12-17 15:56     ` Dmitry Safonov
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-17 15:52 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Kukjin Kim, Hyunki Koo, HYUN-KI KOO, Shinbeom Choi,
	Krzysztof Kozlowski, Jiri Slaby, linux-serial, linux-kernel

On Tue, Dec 17, 2019 at 03:47:34PM +0000, Dmitry Safonov wrote:
> On 12/17/19 2:02 PM, Greg Kroah-Hartman wrote:
> > The samsung_tty driver was trying to abuse the struct uart_port by using
> > two "empty" bytes for its own use.  That's not ok, and was found by
> > removing those fields from the structure.
> > 
> > Move the variables into the port-specific structure, which is where
> > everything else for this port already is.  There is no space wasted here
> > as there was an empty "hole" in the structure already for these bytes.
> 
> Thanks!
> Sorry for not noticing this myself.

You wouldn't have noticed this unless you build for that platform. I
just recently made it buildable for other ones.

> 
> > Cc: Kukjin Kim <kgene@kernel.org>
> > Cc: Hyunki Koo <kkoos00@naver.com>
> > Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
> > Cc: Shinbeom Choi <sbeom.choi@samsung.com>
> > Cc: Krzysztof Kozlowski <krzk@kernel.org>
> > Cc: Dmitry Safonov <dima@arista.com>
> > Cc: Jiri Slaby <jslaby@suse.com>
> > Cc: linux-serial@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> I see you already applied it to your tree, but in case it helps anything:
> Reviewed-by: Dmitry Safonov <dima@arista.com>

The review helps, thanks :)

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields
  2019-12-17 15:52   ` Greg Kroah-Hartman
@ 2019-12-17 15:56     ` Dmitry Safonov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Safonov @ 2019-12-17 15:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kukjin Kim, Hyunki Koo, HYUN-KI KOO, Shinbeom Choi,
	Krzysztof Kozlowski, Jiri Slaby, linux-serial, linux-kernel

On 12/17/19 3:52 PM, Greg Kroah-Hartman wrote:
> On Tue, Dec 17, 2019 at 03:47:34PM +0000, Dmitry Safonov wrote:
>> On 12/17/19 2:02 PM, Greg Kroah-Hartman wrote:
>>> The samsung_tty driver was trying to abuse the struct uart_port by using
>>> two "empty" bytes for its own use.  That's not ok, and was found by
>>> removing those fields from the structure.
>>>
>>> Move the variables into the port-specific structure, which is where
>>> everything else for this port already is.  There is no space wasted here
>>> as there was an empty "hole" in the structure already for these bytes.
>>
>> Thanks!
>> Sorry for not noticing this myself.
> 
> You wouldn't have noticed this unless you build for that platform. I
> just recently made it buildable for other ones.

Ah, I was running CONFIG_COMPILE_TEST and thought that it should trigger
anything (and fixed an issue before sending). Probably, managed not to
enable samsung driver's option.

Thanks,
          Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 14:02 [PATCH] tty: serial: samsung_tty: do not abuse the struct uart_port unused fields Greg Kroah-Hartman
2019-12-17 15:47 ` Dmitry Safonov
2019-12-17 15:52   ` Greg Kroah-Hartman
2019-12-17 15:56     ` Dmitry Safonov

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git