[1/2] i2c: do not leave semaphore armed when copying properties fails

Message ID 20170308184102.29334-1-dmitry.torokhov@gmail.com
State New
Headers show

Commit Message

Dmitry Torokhov March 8, 2017, 6:41 p.m.
We should not leave i2c_register_board_info() early, without unlocking the
__i2c_board_lock.

Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/i2c/i2c-boardinfo.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Wolfram Sang March 9, 2017, 2:48 p.m. | #1
On Wed, Mar 08, 2017 at 10:41:01AM -0800, Dmitry Torokhov wrote:
> We should not leave i2c_register_board_info() early, without unlocking the
> __i2c_board_lock.
> 
> Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

So, it seems that patches 1+2 are related. Because I'd like to have
patch 2 sitting in for-next for a whole cycle for sure, my plan is to
revert the faulty b0c1e95ab44f from for-current and apply the fixed
version (b0c1e95ab44f + this patch squashed) to for-next as well.

Is that okay with you?
Dmitry Torokhov March 9, 2017, 5:38 p.m. | #2
Hi Wolfram,

> On Wed, Mar 08, 2017 at 10:41:01AM -0800, Dmitry Torokhov wrote:
> > We should not leave i2c_register_board_info() early, without unlocking the
> > __i2c_board_lock.
> > 
> > Fixes: b0c1e95ab44f ("i2c: copy device properties when using ...")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> So, it seems that patches 1+2 are related. Because I'd like to have
> patch 2 sitting in for-next for a whole cycle for sure, my plan is to
> revert the faulty b0c1e95ab44f from for-current and apply the fixed
> version (b0c1e95ab44f + this patch squashed) to for-next as well.
> 
> Is that okay with you?

I am perfectly fine with reverting b0c1e95ab44f from for-current,
however I wonder if we could have an immutable branch off 4.11-rc2 (or
-rc1) containing fixed version of patch copying property + patch adding
resources + patch exporting i2c_client_type (I will CC you on that
shortly), which we could share between your tree and mine so I can get
in changes to a few drivers on my side (eeti_ts, atmel, etc).

If you are OK with this I can prepare said branch.

Thanks!
Wolfram Sang March 9, 2017, 11:16 p.m. | #3
> I am perfectly fine with reverting b0c1e95ab44f from for-current,
> however I wonder if we could have an immutable branch off 4.11-rc2 (or

Yes, sure. Please prepare a branch and once I reviewed all patches
touching i2c core, we can (from my side at least) declare it immutable
and I will pull it into i2c. I'll try to review the resources patch this
weekend.
Dmitry Torokhov March 20, 2017, 5:06 p.m. | #4
Hi Wolfram,

On Fri, Mar 10, 2017 at 12:16:53AM +0100, Wolfram Sang wrote:
> 
> > I am perfectly fine with reverting b0c1e95ab44f from for-current,
> > however I wonder if we could have an immutable branch off 4.11-rc2 (or
> 
> Yes, sure. Please prepare a branch and once I reviewed all patches
> touching i2c core, we can (from my side at least) declare it immutable
> and I will pull it into i2c. I'll try to review the resources patch this
> weekend.
> 

Did you have a chance at looking the I2C IRQ resources patch? I prepared
a branch containing the 3 patches:

$ git log --oneline --reverse v4.11-rc3..HEAD
51f19be7f637 i2c: export i2c_client_type structure
5b57e4dd278e i2c: copy device properties when using i2c_register_board_info()
7f4bf035cb84 i2c: allow attaching IRQ resources to i2c_board_info

You can find it at:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git
4.11-rc3-i2c-irq-resources

or:

https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=4.11-rc3-i2c-irq-resources

If you are happy with the branch I'll add your acked-bys and mark it as
immutable so we can share it between our trees.

Thanks!

Patch

diff --git a/drivers/i2c/i2c-boardinfo.c b/drivers/i2c/i2c-boardinfo.c
index 5b8f6c3a6950..0e285c68b2ff 100644
--- a/drivers/i2c/i2c-boardinfo.c
+++ b/drivers/i2c/i2c-boardinfo.c
@@ -84,8 +84,10 @@  int i2c_register_board_info(int busnum, struct i2c_board_info const *info, unsig
 		if (info->properties) {
 			devinfo->board_info.properties =
 					property_entries_dup(info->properties);
-			if (IS_ERR(devinfo->board_info.properties))
-				return PTR_ERR(devinfo->board_info.properties);
+			if (IS_ERR(devinfo->board_info.properties)) {
+				status = PTR_ERR(devinfo->board_info.properties);
+				break;
+			}
 		}
 
 		list_add_tail(&devinfo->list, &__i2c_board_list);