[PATCH] i2c: smbus: fix NULL function pointer dereference

[PATCH] i2c: smbus: fix NULL function pointer dereference

[Wolfram Sang]

Brauch reported an OOPS when using the designware controller as target
only. Target-only modes break the assumption of one transfer function
always being available. Fix this by always checking the pointer in
__i2c_transfer.

Reported-by: Baruch Siach <baruch@tkos.co.il>
Closes: https://lore.kernel.org/r/4269631780e5ba789cf1ae391eec1b959def7d99.1712761976.git.baruch@tkos.co.il
Fixes: 4b1acc43331d ("i2c: core changes for slave support")
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core-base.c  | 12 ++++++------
 drivers/i2c/i2c-core-smbus.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ff5c486a1dbb..db0d1ac82910 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2200,13 +2200,18 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
  * Returns negative errno, else the number of messages executed.
  *
  * Adapter lock must be held when calling this function. No debug logging
- * takes place. adap->algo->master_xfer existence isn't checked.
+ * takes place.
  */
 int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	unsigned long orig_jiffies;
 	int ret, try;
 
+	if (!adap->algo->master_xfer) {
+		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (WARN_ON(!msgs || num < 1))
 		return -EINVAL;
 
@@ -2273,11 +2278,6 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	int ret;
 
-	if (!adap->algo->master_xfer) {
-		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	/* REVISIT the fault reporting model here is weak:
 	 *
 	 *  - When we get an error after receiving N bytes from a slave,
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index e3b96fc53b5c..a942c5306a4e 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -596,7 +596,7 @@ s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 				break;
 		}
 
-		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
+		if (res != -EOPNOTSUPP)
 			goto trace;
 		/*
 		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't
-- 
2.39.2

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Sergei Shtylyov]

On 4/26/24 9:44 AM, Wolfram Sang wrote:

> Brauch reported an OOPS when using the designware controller as target

   He's Baruch... :-)

> only. Target-only modes break the assumption of one transfer function
> always being available. Fix this by always checking the pointer in
> __i2c_transfer.
> 
> Reported-by: Baruch Siach <baruch@tkos.co.il>
> Closes: https://lore.kernel.org/r/4269631780e5ba789cf1ae391eec1b959def7d99.1712761976.git.baruch@tkos.co.il
> Fixes: 4b1acc43331d ("i2c: core changes for slave support")
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[...]

MBR, Sergey

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Wolfram Sang]

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

On Fri, Apr 26, 2024 at 10:28:40AM +0300, Sergei Shtylyov wrote:
> On 4/26/24 9:44 AM, Wolfram Sang wrote:
> 
> > Brauch reported an OOPS when using the designware controller as target
> 
>    He's Baruch... :-)

Oh, I am very sorry Baruch!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Baruch Siach]

Hi Wolfram,

On Fri, Apr 26 2024, Wolfram Sang wrote:
> Brauch reported an OOPS when using the designware controller as target
> only. Target-only modes break the assumption of one transfer function
> always being available. Fix this by always checking the pointer in
> __i2c_transfer.
>
> Reported-by: Baruch Siach <baruch@tkos.co.il>
> Closes: https://lore.kernel.org/r/4269631780e5ba789cf1ae391eec1b959def7d99.1712761976.git.baruch@tkos.co.il
> Fixes: 4b1acc43331d ("i2c: core changes for slave support")
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Tested-by: Baruch Siach <baruch@tkos.co.il>

Thanks,
baruch

> ---
>  drivers/i2c/i2c-core-base.c  | 12 ++++++------
>  drivers/i2c/i2c-core-smbus.c |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index ff5c486a1dbb..db0d1ac82910 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2200,13 +2200,18 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   * Returns negative errno, else the number of messages executed.
>   *
>   * Adapter lock must be held when calling this function. No debug logging
> - * takes place. adap->algo->master_xfer existence isn't checked.
> + * takes place.
>   */
>  int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
>  	unsigned long orig_jiffies;
>  	int ret, try;
>  
> +	if (!adap->algo->master_xfer) {
> +		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	if (WARN_ON(!msgs || num < 1))
>  		return -EINVAL;
>  
> @@ -2273,11 +2278,6 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
>  	int ret;
>  
> -	if (!adap->algo->master_xfer) {
> -		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
> -		return -EOPNOTSUPP;
> -	}
> -
>  	/* REVISIT the fault reporting model here is weak:
>  	 *
>  	 *  - When we get an error after receiving N bytes from a slave,
> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
> index e3b96fc53b5c..a942c5306a4e 100644
> --- a/drivers/i2c/i2c-core-smbus.c
> +++ b/drivers/i2c/i2c-core-smbus.c
> @@ -596,7 +596,7 @@ s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  				break;
>  		}
>  
> -		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
> +		if (res != -EOPNOTSUPP)
>  			goto trace;
>  		/*
>  		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Wolfram Sang]

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

On Fri, Apr 26, 2024 at 08:44:08AM +0200, Wolfram Sang wrote:
> Brauch reported an OOPS when using the designware controller as target
> only. Target-only modes break the assumption of one transfer function
> always being available. Fix this by always checking the pointer in
> __i2c_transfer.
> 
> Reported-by: Baruch Siach <baruch@tkos.co.il>
> Closes: https://lore.kernel.org/r/4269631780e5ba789cf1ae391eec1b959def7d99.1712761976.git.baruch@tkos.co.il
> Fixes: 4b1acc43331d ("i2c: core changes for slave support")
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Fixed the name typo and applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Jean Delvare]

Hi Wolfram, Baruch, Peter,

On Fri, 2024-04-26 at 08:44 +0200, Wolfram Sang wrote:
> Brauch reported an OOPS when using the designware controller as target
> only. Target-only modes break the assumption of one transfer function
> always being available. Fix this by always checking the pointer in
> __i2c_transfer.

I was asked to backport this fix to our oldest kernel branches, so I
looked into it and I have comments and a question.

> Reported-by: Baruch Siach <baruch@tkos.co.il>
> Closes: https://lore.kernel.org/r/4269631780e5ba789cf1ae391eec1b959def7d99.1712761976.git.baruch@tkos.co.il
> Fixes: 4b1acc43331d ("i2c: core changes for slave support")

I have a hard time establishing a formal link between the reported bug
and the commit listed above. I do understand that it wouldn't make
sense to register an i2c_adapter with neither .master_xfer nor
.smbus_xfer set before .reg_slave was added to struct i2c_algorithm,
but there were no checks in i2c-core preventing it from happening.

It was also possible for any (broken) device driver to call
__i2c_transfer() without first checking if plain I2C transfers were
actually supported by the i2c_adapter. I would argue that such an issue
should have been fixed at the device driver level by checking for the
I2C_FUNC_I2C functionality flag before calling __i2c_transfer(). That's
a theoretical issue though as I'm not aware of any device driver having
this issue.

The call stack in Baruch's report shows that the real issue is with
i2c_smbus_xfer_emulated() being called with the i2c bus lock already
held, and thus having to call __i2c_transfer() instead of
i2c_transfer(). This code path did not exist before commit 63453b59e411
("i2c: smbus: add unlocked __i2c_smbus_xfer variant"), which was added
in kernel v4.19. Therefore I claim that CVE-2024-35984 only affects
kernel v4.19 and newer. Do we agree on that?

> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/i2c-core-base.c  | 12 ++++++------
>  drivers/i2c/i2c-core-smbus.c |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index ff5c486a1dbb..db0d1ac82910 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2200,13 +2200,18 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   * Returns negative errno, else the number of messages executed.
>   *
>   * Adapter lock must be held when calling this function. No debug logging
> - * takes place. adap->algo->master_xfer existence isn't checked.
> + * takes place.
>   */
>  int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
>         unsigned long orig_jiffies;
>         int ret, try;
>  
> +       if (!adap->algo->master_xfer) {
> +               dev_dbg(&adap->dev, "I2C level transfers not supported\n");
> +               return -EOPNOTSUPP;
> +       }
> +

Not related specifically to this commit, as it is only moving a check
which already existed before, but this looks inefficient to me.

We end up performing the check with every I2C-level transfer, while the
availability of such support can almost always be checked once and for
all in the I2C device driver (i2c-dev and i2c_smbus_xfer_emulated being
the exceptions).

I see two ways for us to reach this check:
* __i2c_transfer() or i2c_transfer() gets called directly by a device
driver. This driver should have checked for the I2C_FUNC_I2C
functionality flag before calling either function. If they did not,
it's a driver bug, which should be fixed in the driver in question.
Note that i2c-dev currently lacks this check, I think it should be
added.
* __i2c_transfer() gets called by i2c_smbus_xfer_emulated(). We should
add a check for I2C_FUNC_I2C in __i2c_smbus_xfer() before calling this
function. This is more or less what Baruch proposed initially, and I
think it would have been a more efficient fix.

And if you are concerned about functionality flags not being set
properly (specifically I2C_FUNC_I2C being set while .master_xfer isn't
set [1]) then we should add a consistency check at i2c_adapter
registration time, so again it's done once and for all and we don't
have to check again and again at transfer time.

Or is this optimization not worth it?

[1] BTW, looking at the only two in-tree slave-only I2C adapter
drivers, i2c-at91-slave and i2c-designware-slave, both are setting
functionality flags other than I2C_FUNC_SLAVE. Unless I don't
understand how the slave functionality works, this is a bug. I'll
prepare and post patches later today.


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Wolfram Sang]

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

Hi Jean,

> I have a hard time establishing a formal link between the reported bug
> and the commit listed above. I do understand that it wouldn't make
> sense to register an i2c_adapter with neither .master_xfer nor
> .smbus_xfer set before .reg_slave was added to struct i2c_algorithm,
> but there were no checks in i2c-core preventing it from happening.

Well, yes, correct.

> It was also possible for any (broken) device driver to call
> __i2c_transfer() without first checking if plain I2C transfers were
> actually supported by the i2c_adapter. I would argue that such an issue
> should have been fixed at the device driver level by checking for the
> I2C_FUNC_I2C functionality flag before calling __i2c_transfer(). That's
> a theoretical issue though as I'm not aware of any device driver having
> this issue.

In theory, checking against I2C_FUNC_I2C should happen. In practice,
most I2C drivers do not do this. Being picky here could results in bad
user experience because of OOPS. If we really want to enforce checking
I2C_FUNC_I2C, then we should have this safety net while we convert all
users. No, actually, I think we always should have some safety nets.

> The call stack in Baruch's report shows that the real issue is with
> i2c_smbus_xfer_emulated() being called with the i2c bus lock already
> held, and thus having to call __i2c_transfer() instead of
> i2c_transfer(). This code path did not exist before commit 63453b59e411
> ("i2c: smbus: add unlocked __i2c_smbus_xfer variant"), which was added
> in kernel v4.19. Therefore I claim that CVE-2024-35984 only affects
> kernel v4.19 and newer. Do we agree on that?

(There is a CVE for it??) For Baruch's case, this is true. But there are
__i2c_transfer users all over the tree, they are all potentially
vulnerable, or?

> > +       if (!adap->algo->master_xfer) {
> > +               dev_dbg(&adap->dev, "I2C level transfers not supported\n");
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> 
> Not related specifically to this commit, as it is only moving a check
> which already existed before, but this looks inefficient to me.
> 
> We end up performing the check with every I2C-level transfer, while the
> availability of such support can almost always be checked once and for
> all in the I2C device driver (i2c-dev and i2c_smbus_xfer_emulated being
> the exceptions).
> 
> I see two ways for us to reach this check:
> * __i2c_transfer() or i2c_transfer() gets called directly by a device
> driver. This driver should have checked for the I2C_FUNC_I2C
> functionality flag before calling either function. If they did not,
> it's a driver bug, which should be fixed in the driver in question.

I see the performance penalty, yet I prefer handling the buggy driver
gracefully because kicking off I2C transfers is not a hot path. Maybe we
could turn the dev_dbg into something louder to make people aware that
there is a bug?

> Note that i2c-dev currently lacks this check, I think it should be
> added.

True, it should be a role-model of a good citizen :)

> * __i2c_transfer() gets called by i2c_smbus_xfer_emulated(). We should
> add a check for I2C_FUNC_I2C in __i2c_smbus_xfer() before calling this
> function. This is more or less what Baruch proposed initially, and I
> think it would have been a more efficient fix.

As I said above, more efficient but not thorough. One driver not
checking I2C_FUNC_I2C and boom...

> And if you are concerned about functionality flags not being set
> properly (specifically I2C_FUNC_I2C being set while .master_xfer isn't
> set [1]) then we should add a consistency check at i2c_adapter
> registration time, so again it's done once and for all and we don't
> have to check again and again at transfer time.

I think this check is worth to have. It is not complete because drivers
may still not check the flag, but it is one step to be more robust.

> Or is this optimization not worth it?

I think so. It is one pointer check against a kernel oopsing somewhere
somewhen.

> [1] BTW, looking at the only two in-tree slave-only I2C adapter
> drivers, i2c-at91-slave and i2c-designware-slave, both are setting
> functionality flags other than I2C_FUNC_SLAVE. Unless I don't
> understand how the slave functionality works, this is a bug. I'll
> prepare and post patches later today.

Thanks for doing that!

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Jean Delvare]

Hi Wolfram,

Thanks for your answer.

On Tue, 4 Jun 2024 10:50:30 +0200, Wolfram Sang wrote:
> Hi Jean,
> 
> > I have a hard time establishing a formal link between the reported bug
> > and the commit listed above. I do understand that it wouldn't make
> > sense to register an i2c_adapter with neither .master_xfer nor
> > .smbus_xfer set before .reg_slave was added to struct i2c_algorithm,
> > but there were no checks in i2c-core preventing it from happening.  
> 
> Well, yes, correct.
> 
> > It was also possible for any (broken) device driver to call
> > __i2c_transfer() without first checking if plain I2C transfers were
> > actually supported by the i2c_adapter. I would argue that such an issue
> > should have been fixed at the device driver level by checking for the
> > I2C_FUNC_I2C functionality flag before calling __i2c_transfer(). That's
> > a theoretical issue though as I'm not aware of any device driver having
> > this issue.  
> 
> In theory, checking against I2C_FUNC_I2C should happen. In practice,
> most I2C drivers do not do this. Being picky here could results in bad
> user experience because of OOPS. If we really want to enforce checking
> I2C_FUNC_I2C, then we should have this safety net while we convert all
> users. No, actually, I think we always should have some safety nets.

Point taken, makes sense.

Note that we still want I2C_FUNC_I2C to be set properly, because it
allows device drivers to optimize transfers (the at24 driver is a prime
example of that) or even just to bind to the I2C bus (for device
drivers which properly check for it).

> > The call stack in Baruch's report shows that the real issue is with
> > i2c_smbus_xfer_emulated() being called with the i2c bus lock already
> > held, and thus having to call __i2c_transfer() instead of
> > i2c_transfer(). This code path did not exist before commit 63453b59e411
> > ("i2c: smbus: add unlocked __i2c_smbus_xfer variant"), which was added
> > in kernel v4.19. Therefore I claim that CVE-2024-35984 only affects
> > kernel v4.19 and newer. Do we agree on that?  
> 
> (There is a CVE for it??) For Baruch's case, this is true. But there are
> __i2c_transfer users all over the tree, they are all potentially
> vulnerable, or?

Yes there are many, but I think we shall differentiate between 2 cases:
* Missing check in a specific kernel device driver. These are unlikely
  to be a problem in practice because (1) these devices are typically
  instantiated explicitly, and such explicit code or device tree
  description would not exist in the first place if said device was not
  compatible with said I2C bus, and (2) if such an incompatibility was
  really present then it would have been spotted and fixed very
  quickly. Arbitrary binding through sysfs attributes is still possible
  but would definitely require root access and evil intentions (at
  which point we are screwed no matter what). I'm honestly not worried
  about this scenario.
* The issue being triggered from user-space through i2c-dev, which is
  what Baruch reported. The user doing that can target any arbitrary
  I2C bus and thus cause the oops by accident or even on purpose. For
  me this is what CVE-2024-35984 is about. What limits the attack
  surface here is that slave-only I2C buses are rare and you typically
  need to be root to use i2c-dev. But this is still a serious issue.

Also note that the first case could happen ever since __i2c_transfer()
was introduced (kernel v3.6, commit b37d2a3a75cb) and is not limited to
slave-only adapters, as any SMBus-only i2c_adapter would also be
vulnerable.

So the "Fixes:" tag in commit 91811a31b68d is incorrect for both
scenarios.

> (...)
> I see the performance penalty, yet I prefer handling the buggy driver
> gracefully because kicking off I2C transfers is not a hot path. Maybe we
> could turn the dev_dbg into something louder to make people aware that
> there is a bug?

My previous message initially had a suggestion in that direction ;-)
but I first wanted your opinion on the check itself. dev_dbg() is
definitely not appropriate for a condition which should never happen
and implies there's a bug somewhere else. A WARN_ON_ONCE would probably
be better, so that the bug gets spotted and fixed quickly.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Wolfram Sang]

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

Hi Jean,

> Note that we still want I2C_FUNC_I2C to be set properly, because it
> allows device drivers to optimize transfers (the at24 driver is a prime
> example of that) or even just to bind to the I2C bus (for device
> drivers which properly check for it).

I agree. We definitely want I2C_FUNC_I2C to be set and make use of it as
much as possible. We should just not completely rely on it.

> > (There is a CVE for it??) For Baruch's case, this is true. But there are
> > __i2c_transfer users all over the tree, they are all potentially
> > vulnerable, or?
> 
> Yes there are many, but I think we shall differentiate between 2 cases:
> * Missing check in a specific kernel device driver. These are unlikely
>   to be a problem in practice because (1) these devices are typically
>   instantiated explicitly, and such explicit code or device tree
>   description would not exist in the first place if said device was not
>   compatible with said I2C bus, and (2) if such an incompatibility was
>   really present then it would have been spotted and fixed very
>   quickly. Arbitrary binding through sysfs attributes is still possible
>   but would definitely require root access and evil intentions (at
>   which point we are screwed no matter what). I'm honestly not worried
>   about this scenario.

OK, can be argued.

> * The issue being triggered from user-space through i2c-dev, which is
>   what Baruch reported. The user doing that can target any arbitrary
>   I2C bus and thus cause the oops by accident or even on purpose. For
>   me this is what CVE-2024-35984 is about. What limits the attack
>   surface here is that slave-only I2C buses are rare and you typically
>   need to be root to use i2c-dev. But this is still a serious issue.

Agreed.

> Also note that the first case could happen ever since __i2c_transfer()
> was introduced (kernel v3.6, commit b37d2a3a75cb) and is not limited to
> slave-only adapters, as any SMBus-only i2c_adapter would also be
> vulnerable.

Which makes handling this gracefully even more important.

> So the "Fixes:" tag in commit 91811a31b68d is incorrect for both
> scenarios.

Ack. Sorry! :)

> > gracefully because kicking off I2C transfers is not a hot path. Maybe we
> > could turn the dev_dbg into something louder to make people aware that
> > there is a bug?
> 
> My previous message initially had a suggestion in that direction ;-)
> but I first wanted your opinion on the check itself. dev_dbg() is
> definitely not appropriate for a condition which should never happen
> and implies there's a bug somewhere else. A WARN_ON_ONCE would probably
> be better, so that the bug gets spotted and fixed quickly.

So, are you okay with keeping the check where it is now and turning the
dev_dbg into WARN_ON_ONCE? I am.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: smbus: fix NULL function pointer dereference

[Jean Delvare]

Hi Wolfram,

On Tue, 4 Jun 2024 22:08:41 +0200, Wolfram Sang wrote:
> > > gracefully because kicking off I2C transfers is not a hot path. Maybe we
> > > could turn the dev_dbg into something louder to make people aware that
> > > there is a bug?  
> > 
> > My previous message initially had a suggestion in that direction ;-)
> > but I first wanted your opinion on the check itself. dev_dbg() is
> > definitely not appropriate for a condition which should never happen
> > and implies there's a bug somewhere else. A WARN_ON_ONCE would probably
> > be better, so that the bug gets spotted and fixed quickly.  
> 
> So, are you okay with keeping the check where it is now and turning the
> dev_dbg into WARN_ON_ONCE? I am.

Yes I am.

Thanks,
-- 
Jean Delvare
SUSE L3 Support