[PATCH stable 5.10] serial: sc16is7xx: convert from _raw_ to _noinc_ regmap functions for FIFO

[PATCH stable 5.10] serial: sc16is7xx: convert from _raw_ to _noinc_ regmap functions for FIFO

[GONG, Ruiqi]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit dbf4ab821804df071c8b566d9813083125e6d97b upstream.

The SC16IS7XX IC supports a burst mode to access the FIFOs where the
initial register address is sent ($00), followed by all the FIFO data
without having to resend the register address each time. In this mode, the
IC doesn't increment the register address for each R/W byte.

The regmap_raw_read() and regmap_raw_write() are functions which can
perform IO over multiple registers. They are currently used to read/write
from/to the FIFO, and although they operate correctly in this burst mode on
the SPI bus, they would corrupt the regmap cache if it was not disabled
manually. The reason is that when the R/W size is more than 1 byte, these
functions assume that the register address is incremented and handle the
cache accordingly.

Convert FIFO R/W functions to use the regmap _noinc_ versions in order to
remove the manual cache control which was a workaround when using the
_raw_ versions. FIFO registers are properly declared as volatile so
cache will not be used/updated for FIFO accesses.

Fixes: dfeae619d781 ("serial: sc16is7xx")
Cc:  <stable@vger.kernel.org>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231211171353.2901416-6-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
---

The mainline commit dbf4ab821804 ("serial: sc16is7xx: convert from _raw_
to _noinc_ regmap functions for FIFO") by Hugo has been assigned to be
CVE-2023-52488, but for stable branches lower than 6.1 there's no
official backport.

I made up this backport patch for 5.10, and its correctness has been
confirmed in previous communication with Hugo. Let's publicize it and
merge it into upstream.

 drivers/tty/serial/sc16is7xx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 31e0c5c3ddea..29f05db0d49b 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -376,9 +376,7 @@ static void sc16is7xx_fifo_read(struct uart_port *port, unsigned int rxlen)
 	const u8 line = sc16is7xx_line(port);
 	u8 addr = (SC16IS7XX_RHR_REG << SC16IS7XX_REG_SHIFT) | line;
 
-	regcache_cache_bypass(s->regmap, true);
-	regmap_raw_read(s->regmap, addr, s->buf, rxlen);
-	regcache_cache_bypass(s->regmap, false);
+	regmap_noinc_read(s->regmap, addr, s->buf, rxlen);
 }
 
 static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
@@ -394,9 +392,7 @@ static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
 	if (unlikely(!to_send))
 		return;
 
-	regcache_cache_bypass(s->regmap, true);
-	regmap_raw_write(s->regmap, addr, s->buf, to_send);
-	regcache_cache_bypass(s->regmap, false);
+	regmap_noinc_write(s->regmap, addr, s->buf, to_send);
 }
 
 static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
@@ -489,6 +485,11 @@ static bool sc16is7xx_regmap_precious(struct device *dev, unsigned int reg)
 	return false;
 }
 
+static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
+{
+	return reg == SC16IS7XX_RHR_REG;
+}
+
 static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
@@ -1439,6 +1440,8 @@ static struct regmap_config regcfg = {
 	.cache_type = REGCACHE_RBTREE,
 	.volatile_reg = sc16is7xx_regmap_volatile,
 	.precious_reg = sc16is7xx_regmap_precious,
+	.writeable_noinc_reg = sc16is7xx_regmap_noinc,
+	.readable_noinc_reg = sc16is7xx_regmap_noinc,
 };
 
 #ifdef CONFIG_SERIAL_SC16IS7XX_SPI
-- 
2.25.1

Re: [PATCH stable 5.10] serial: sc16is7xx: convert from _raw_ to _noinc_ regmap functions for FIFO

[Gong Ruiqi]

Oops. + Cc stable@vger.kernel.org

On 2024/03/18 10:52, GONG, Ruiqi wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> commit dbf4ab821804df071c8b566d9813083125e6d97b upstream.
> 
> The SC16IS7XX IC supports a burst mode to access the FIFOs where the
> initial register address is sent ($00), followed by all the FIFO data
> without having to resend the register address each time. In this mode, the
> IC doesn't increment the register address for each R/W byte.
> 
> The regmap_raw_read() and regmap_raw_write() are functions which can
> perform IO over multiple registers. They are currently used to read/write
> from/to the FIFO, and although they operate correctly in this burst mode on
> the SPI bus, they would corrupt the regmap cache if it was not disabled
> manually. The reason is that when the R/W size is more than 1 byte, these
> functions assume that the register address is incremented and handle the
> cache accordingly.
> 
> Convert FIFO R/W functions to use the regmap _noinc_ versions in order to
> remove the manual cache control which was a workaround when using the
> _raw_ versions. FIFO registers are properly declared as volatile so
> cache will not be used/updated for FIFO accesses.
> 
> Fixes: dfeae619d781 ("serial: sc16is7xx")
> Cc:  <stable@vger.kernel.org>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Link: https://lore.kernel.org/r/20231211171353.2901416-6-hugo@hugovil.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> ---
> 
> The mainline commit dbf4ab821804 ("serial: sc16is7xx: convert from _raw_
> to _noinc_ regmap functions for FIFO") by Hugo has been assigned to be
> CVE-2023-52488, but for stable branches lower than 6.1 there's no
> official backport.
> 
> I made up this backport patch for 5.10, and its correctness has been
> confirmed in previous communication with Hugo. Let's publicize it and
> merge it into upstream.
> 
>  drivers/tty/serial/sc16is7xx.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 31e0c5c3ddea..29f05db0d49b 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -376,9 +376,7 @@ static void sc16is7xx_fifo_read(struct uart_port *port, unsigned int rxlen)
>  	const u8 line = sc16is7xx_line(port);
>  	u8 addr = (SC16IS7XX_RHR_REG << SC16IS7XX_REG_SHIFT) | line;
>  
> -	regcache_cache_bypass(s->regmap, true);
> -	regmap_raw_read(s->regmap, addr, s->buf, rxlen);
> -	regcache_cache_bypass(s->regmap, false);
> +	regmap_noinc_read(s->regmap, addr, s->buf, rxlen);
>  }
>  
>  static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
> @@ -394,9 +392,7 @@ static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
>  	if (unlikely(!to_send))
>  		return;
>  
> -	regcache_cache_bypass(s->regmap, true);
> -	regmap_raw_write(s->regmap, addr, s->buf, to_send);
> -	regcache_cache_bypass(s->regmap, false);
> +	regmap_noinc_write(s->regmap, addr, s->buf, to_send);
>  }
>  
>  static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
> @@ -489,6 +485,11 @@ static bool sc16is7xx_regmap_precious(struct device *dev, unsigned int reg)
>  	return false;
>  }
>  
> +static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
> +{
> +	return reg == SC16IS7XX_RHR_REG;
> +}
> +
>  static int sc16is7xx_set_baud(struct uart_port *port, int baud)
>  {
>  	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
> @@ -1439,6 +1440,8 @@ static struct regmap_config regcfg = {
>  	.cache_type = REGCACHE_RBTREE,
>  	.volatile_reg = sc16is7xx_regmap_volatile,
>  	.precious_reg = sc16is7xx_regmap_precious,
> +	.writeable_noinc_reg = sc16is7xx_regmap_noinc,
> +	.readable_noinc_reg = sc16is7xx_regmap_noinc,
>  };
>  
>  #ifdef CONFIG_SERIAL_SC16IS7XX_SPI

Re: [PATCH stable 5.10] serial: sc16is7xx: convert from _raw_ to _noinc_ regmap functions for FIFO

[Greg Kroah-Hartman]

On Mon, Mar 18, 2024 at 11:14:13AM +0800, Gong Ruiqi wrote:
> Oops. + Cc stable@vger.kernel.org
> 
> On 2024/03/18 10:52, GONG, Ruiqi wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > commit dbf4ab821804df071c8b566d9813083125e6d97b upstream.
> > 
> > The SC16IS7XX IC supports a burst mode to access the FIFOs where the
> > initial register address is sent ($00), followed by all the FIFO data
> > without having to resend the register address each time. In this mode, the
> > IC doesn't increment the register address for each R/W byte.
> > 
> > The regmap_raw_read() and regmap_raw_write() are functions which can
> > perform IO over multiple registers. They are currently used to read/write
> > from/to the FIFO, and although they operate correctly in this burst mode on
> > the SPI bus, they would corrupt the regmap cache if it was not disabled
> > manually. The reason is that when the R/W size is more than 1 byte, these
> > functions assume that the register address is incremented and handle the
> > cache accordingly.
> > 
> > Convert FIFO R/W functions to use the regmap _noinc_ versions in order to
> > remove the manual cache control which was a workaround when using the
> > _raw_ versions. FIFO registers are properly declared as volatile so
> > cache will not be used/updated for FIFO accesses.
> > 
> > Fixes: dfeae619d781 ("serial: sc16is7xx")
> > Cc:  <stable@vger.kernel.org>
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > Link: https://lore.kernel.org/r/20231211171353.2901416-6-hugo@hugovil.com
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> > ---
> > 
> > The mainline commit dbf4ab821804 ("serial: sc16is7xx: convert from _raw_
> > to _noinc_ regmap functions for FIFO") by Hugo has been assigned to be
> > CVE-2023-52488, but for stable branches lower than 6.1 there's no
> > official backport.
> > 
> > I made up this backport patch for 5.10, and its correctness has been
> > confirmed in previous communication with Hugo. Let's publicize it and
> > merge it into upstream.

I can not take this only in 5.10, it needs to also go into 5.15.y first,
right?

Please resend a 5.15.y and this 5.10.y version when you have both of
them (the 5.10.y version wasn't sent to stable@k.o so it's hard to track
down), and we will be glad to take them both.

thanks,

greg k-h