* [PATCH 0/3] mcb changes for 5.14
@ 2021-06-15 14:55 Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code Johannes Thumshirn
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2021-06-15 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Johannes Thumshirn
Hi Greg,
Here are the collected patches for mcb for 5.14.
The following changes since commit 009c9aa5be652675a06d5211e1640e02bbb1c33d:
Linux 5.13-rc6 (2021-06-13 14:43:10 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git mcb-for-5.14
for you to fetch changes up to fe520620eeddaa2ed8c669125f9b673c89d6b5a5:
mcb: Remove trailing semicolon in macros (2021-06-15 23:43:47 +0900)
----------------------------------------------------------------
Alternatively the broken out patches are in this thread.
Dan Carpenter (1):
mcb: fix error handling in mcb_alloc_bus()
Huilong Deng (1):
mcb: Remove trailing semicolon in macros
Zhen Lei (1):
mcb: Use DEFINE_RES_MEM() to simplify code
drivers/mcb/mcb-core.c | 12 ++++++------
drivers/mcb/mcb-lpc.c | 13 ++-----------
include/linux/mcb.h | 2 +-
3 files changed, 9 insertions(+), 18 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code
2021-06-15 14:55 [PATCH 0/3] mcb changes for 5.14 Johannes Thumshirn
@ 2021-06-15 14:55 ` Johannes Thumshirn
2021-06-15 15:42 ` Greg Kroah-Hartman
2021-06-15 14:55 ` [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus() Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 3/3] mcb: Remove trailing semicolon in macros Johannes Thumshirn
2 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2021-06-15 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Johannes Thumshirn, Zhen Lei, Johannes Thumshirn
From: Zhen Lei <thunder.leizhen@huawei.com>
The value of '.end' should be "start + size - 1". So the previous writing
should have omitted subtracted 1.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
drivers/mcb/mcb-lpc.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/mcb/mcb-lpc.c b/drivers/mcb/mcb-lpc.c
index 506676754538..53decd89876e 100644
--- a/drivers/mcb/mcb-lpc.c
+++ b/drivers/mcb/mcb-lpc.c
@@ -105,17 +105,8 @@ static int mcb_lpc_create_platform_device(const struct dmi_system_id *id)
return ret;
}
-static struct resource sc24_fpga_resource = {
- .start = 0xe000e000,
- .end = 0xe000e000 + CHAM_HEADER_SIZE,
- .flags = IORESOURCE_MEM,
-};
-
-static struct resource sc31_fpga_resource = {
- .start = 0xf000e000,
- .end = 0xf000e000 + CHAM_HEADER_SIZE,
- .flags = IORESOURCE_MEM,
-};
+static struct resource sc24_fpga_resource = DEFINE_RES_MEM(0xe000e000, CHAM_HEADER_SIZE);
+static struct resource sc31_fpga_resource = DEFINE_RES_MEM(0xf000e000, CHAM_HEADER_SIZE);
static struct platform_driver mcb_lpc_driver = {
.driver = {
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus()
2021-06-15 14:55 [PATCH 0/3] mcb changes for 5.14 Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code Johannes Thumshirn
@ 2021-06-15 14:55 ` Johannes Thumshirn
2021-06-15 15:41 ` Greg Kroah-Hartman
2021-06-15 14:55 ` [PATCH 3/3] mcb: Remove trailing semicolon in macros Johannes Thumshirn
2 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2021-06-15 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Johannes Thumshirn, Dan Carpenter, Johannes Thumshirn
From: Dan Carpenter <dan.carpenter@oracle.com>
There are two bugs:
1) If ida_simple_get() fails then this code calls put_device(carrier)
but we haven't yet called get_device(carrier) and probably that
leads to a use after free.
2) After device_initialize() then we need to use put_device() to
release the bus. This will free the internal resources tied to the
device and call mcb_free_bus() which will free the rest.
Fixes: 5d9e2ab9fea4 ("mcb: Implement bus->dev.release callback")
Fixes: 18d288198099 ("mcb: Correctly initialize the bus's device")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
drivers/mcb/mcb-core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mcb/mcb-core.c b/drivers/mcb/mcb-core.c
index 38fbb3b59873..38cc8340e817 100644
--- a/drivers/mcb/mcb-core.c
+++ b/drivers/mcb/mcb-core.c
@@ -277,8 +277,8 @@ struct mcb_bus *mcb_alloc_bus(struct device *carrier)
bus_nr = ida_simple_get(&mcb_ida, 0, 0, GFP_KERNEL);
if (bus_nr < 0) {
- rc = bus_nr;
- goto err_free;
+ kfree(bus);
+ return ERR_PTR(bus_nr);
}
bus->bus_nr = bus_nr;
@@ -293,12 +293,12 @@ struct mcb_bus *mcb_alloc_bus(struct device *carrier)
dev_set_name(&bus->dev, "mcb:%d", bus_nr);
rc = device_add(&bus->dev);
if (rc)
- goto err_free;
+ goto err_put;
return bus;
-err_free:
- put_device(carrier);
- kfree(bus);
+
+err_put:
+ put_device(&bus->dev);
return ERR_PTR(rc);
}
EXPORT_SYMBOL_NS_GPL(mcb_alloc_bus, MCB);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] mcb: Remove trailing semicolon in macros
2021-06-15 14:55 [PATCH 0/3] mcb changes for 5.14 Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus() Johannes Thumshirn
@ 2021-06-15 14:55 ` Johannes Thumshirn
2021-06-15 15:48 ` Greg Kroah-Hartman
2 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2021-06-15 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Johannes Thumshirn, Huilong Deng, Johannes Thumshirn
From: Huilong Deng <denghuilong@cdjrlc.com>
Macros should not use a trailing semicolon.
Signed-off-by: Huilong Deng <denghuilong@cdjrlc.com>
Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
include/linux/mcb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mcb.h b/include/linux/mcb.h
index 71dd10a3d928..f6efb16f9d1b 100644
--- a/include/linux/mcb.h
+++ b/include/linux/mcb.h
@@ -120,7 +120,7 @@ extern int __must_check __mcb_register_driver(struct mcb_driver *drv,
__mcb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
extern void mcb_unregister_driver(struct mcb_driver *driver);
#define module_mcb_driver(__mcb_driver) \
- module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver);
+ module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver)
extern void mcb_bus_add_devices(const struct mcb_bus *bus);
extern int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev);
extern struct mcb_bus *mcb_alloc_bus(struct device *carrier);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus()
2021-06-15 14:55 ` [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus() Johannes Thumshirn
@ 2021-06-15 15:41 ` Greg Kroah-Hartman
2021-06-15 15:44 ` Johannes Thumshirn
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-15 15:41 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-kernel, Dan Carpenter, Johannes Thumshirn
On Tue, Jun 15, 2021 at 11:55:29PM +0900, Johannes Thumshirn wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
>
> There are two bugs:
> 1) If ida_simple_get() fails then this code calls put_device(carrier)
> but we haven't yet called get_device(carrier) and probably that
> leads to a use after free.
> 2) After device_initialize() then we need to use put_device() to
> release the bus. This will free the internal resources tied to the
> device and call mcb_free_bus() which will free the rest.
>
> Fixes: 5d9e2ab9fea4 ("mcb: Implement bus->dev.release callback")
> Fixes: 18d288198099 ("mcb: Correctly initialize the bus's device")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
> ---
> drivers/mcb/mcb-core.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Shouldn't this go to the stable kernels? Why not cc: stable in the
signed-off-by lines?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code
2021-06-15 14:55 ` [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code Johannes Thumshirn
@ 2021-06-15 15:42 ` Greg Kroah-Hartman
2021-06-16 1:31 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-15 15:42 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-kernel, Zhen Lei, Johannes Thumshirn
On Tue, Jun 15, 2021 at 11:55:28PM +0900, Johannes Thumshirn wrote:
> From: Zhen Lei <thunder.leizhen@huawei.com>
>
> The value of '.end' should be "start + size - 1". So the previous writing
> should have omitted subtracted 1.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
> ---
> drivers/mcb/mcb-lpc.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mcb/mcb-lpc.c b/drivers/mcb/mcb-lpc.c
> index 506676754538..53decd89876e 100644
> --- a/drivers/mcb/mcb-lpc.c
> +++ b/drivers/mcb/mcb-lpc.c
> @@ -105,17 +105,8 @@ static int mcb_lpc_create_platform_device(const struct dmi_system_id *id)
> return ret;
> }
>
> -static struct resource sc24_fpga_resource = {
> - .start = 0xe000e000,
> - .end = 0xe000e000 + CHAM_HEADER_SIZE,
> - .flags = IORESOURCE_MEM,
> -};
> -
> -static struct resource sc31_fpga_resource = {
> - .start = 0xf000e000,
> - .end = 0xf000e000 + CHAM_HEADER_SIZE,
> - .flags = IORESOURCE_MEM,
> -};
> +static struct resource sc24_fpga_resource = DEFINE_RES_MEM(0xe000e000, CHAM_HEADER_SIZE);
> +static struct resource sc31_fpga_resource = DEFINE_RES_MEM(0xf000e000, CHAM_HEADER_SIZE);
Does this simplify stuff, or does it fix a bug with the existing
definition?
Your changelog text says one thing, but your subject says the other...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus()
2021-06-15 15:41 ` Greg Kroah-Hartman
@ 2021-06-15 15:44 ` Johannes Thumshirn
0 siblings, 0 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2021-06-15 15:44 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Dan Carpenter, Johannes Thumshirn
On 15/06/2021 17:42, Greg Kroah-Hartman wrote:
> On Tue, Jun 15, 2021 at 11:55:29PM +0900, Johannes Thumshirn wrote:
>> From: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> There are two bugs:
>> 1) If ida_simple_get() fails then this code calls put_device(carrier)
>> but we haven't yet called get_device(carrier) and probably that
>> leads to a use after free.
>> 2) After device_initialize() then we need to use put_device() to
>> release the bus. This will free the internal resources tied to the
>> device and call mcb_free_bus() which will free the rest.
>>
>> Fixes: 5d9e2ab9fea4 ("mcb: Implement bus->dev.release callback")
>> Fixes: 18d288198099 ("mcb: Correctly initialize the bus's device")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
>> ---
>> drivers/mcb/mcb-core.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> Shouldn't this go to the stable kernels? Why not cc: stable in the
> signed-off-by lines?
Right, I'll add a cc stable
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] mcb: Remove trailing semicolon in macros
2021-06-15 14:55 ` [PATCH 3/3] mcb: Remove trailing semicolon in macros Johannes Thumshirn
@ 2021-06-15 15:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-15 15:48 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-kernel, Huilong Deng, Johannes Thumshirn
On Tue, Jun 15, 2021 at 11:55:30PM +0900, Johannes Thumshirn wrote:
> From: Huilong Deng <denghuilong@cdjrlc.com>
>
> Macros should not use a trailing semicolon.
>
> Signed-off-by: Huilong Deng <denghuilong@cdjrlc.com>
> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
> ---
> include/linux/mcb.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mcb.h b/include/linux/mcb.h
> index 71dd10a3d928..f6efb16f9d1b 100644
> --- a/include/linux/mcb.h
> +++ b/include/linux/mcb.h
> @@ -120,7 +120,7 @@ extern int __must_check __mcb_register_driver(struct mcb_driver *drv,
> __mcb_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
> extern void mcb_unregister_driver(struct mcb_driver *driver);
> #define module_mcb_driver(__mcb_driver) \
> - module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver);
> + module_driver(__mcb_driver, mcb_register_driver, mcb_unregister_driver)
> extern void mcb_bus_add_devices(const struct mcb_bus *bus);
> extern int mcb_device_register(struct mcb_bus *bus, struct mcb_device *dev);
> extern struct mcb_bus *mcb_alloc_bus(struct device *carrier);
> --
> 2.31.1
>
I've applied this one, feel free to resend the first 2 as emails.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code
2021-06-15 15:42 ` Greg Kroah-Hartman
@ 2021-06-16 1:31 ` Leizhen (ThunderTown)
2021-06-16 6:41 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Leizhen (ThunderTown) @ 2021-06-16 1:31 UTC (permalink / raw)
To: Greg Kroah-Hartman, Johannes Thumshirn; +Cc: linux-kernel, Johannes Thumshirn
On 2021/6/15 23:42, Greg Kroah-Hartman wrote:
> On Tue, Jun 15, 2021 at 11:55:28PM +0900, Johannes Thumshirn wrote:
>> From: Zhen Lei <thunder.leizhen@huawei.com>
>>
>> The value of '.end' should be "start + size - 1". So the previous writing
>> should have omitted subtracted 1.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
>> ---
>> drivers/mcb/mcb-lpc.c | 13 ++-----------
>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mcb/mcb-lpc.c b/drivers/mcb/mcb-lpc.c
>> index 506676754538..53decd89876e 100644
>> --- a/drivers/mcb/mcb-lpc.c
>> +++ b/drivers/mcb/mcb-lpc.c
>> @@ -105,17 +105,8 @@ static int mcb_lpc_create_platform_device(const struct dmi_system_id *id)
>> return ret;
>> }
>>
>> -static struct resource sc24_fpga_resource = {
>> - .start = 0xe000e000,
>> - .end = 0xe000e000 + CHAM_HEADER_SIZE,
>> - .flags = IORESOURCE_MEM,
>> -};
>> -
>> -static struct resource sc31_fpga_resource = {
>> - .start = 0xf000e000,
>> - .end = 0xf000e000 + CHAM_HEADER_SIZE,
>> - .flags = IORESOURCE_MEM,
>> -};
>> +static struct resource sc24_fpga_resource = DEFINE_RES_MEM(0xe000e000, CHAM_HEADER_SIZE);
>> +static struct resource sc31_fpga_resource = DEFINE_RES_MEM(0xf000e000, CHAM_HEADER_SIZE);
>
> Does this simplify stuff, or does it fix a bug with the existing
> definition?
It does not fix a bug, the actual value of .end should be "0xe000e000 + CHAM_HEADER_SIZE - 1".
There is no functional problem, just a little more memory. So it's just been corrected in the
process of simplification.
Do you think it's necessary to split it into two patches?
>
> Your changelog text says one thing, but your subject says the other...
Okay, I'll change the description. How about:
Use DEFINE_RES_MEM() to save a couple of lines of code, which makes the
code a bit shorter and easier to read. The start address does not need to
appear twice.
By the way, the value of '.end' should be "start + size - 1". So the
previous writing should have omitted subtracted 1.
>
> thanks,
>
> greg k-h
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code
2021-06-16 1:31 ` Leizhen (ThunderTown)
@ 2021-06-16 6:41 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-16 6:41 UTC (permalink / raw)
To: Leizhen (ThunderTown)
Cc: Johannes Thumshirn, linux-kernel, Johannes Thumshirn
On Wed, Jun 16, 2021 at 09:31:37AM +0800, Leizhen (ThunderTown) wrote:
>
>
> On 2021/6/15 23:42, Greg Kroah-Hartman wrote:
> > On Tue, Jun 15, 2021 at 11:55:28PM +0900, Johannes Thumshirn wrote:
> >> From: Zhen Lei <thunder.leizhen@huawei.com>
> >>
> >> The value of '.end' should be "start + size - 1". So the previous writing
> >> should have omitted subtracted 1.
> >>
> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >> Signed-off-by: Johannes Thumshirn <jth@kernel.org>
> >> ---
> >> drivers/mcb/mcb-lpc.c | 13 ++-----------
> >> 1 file changed, 2 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/mcb/mcb-lpc.c b/drivers/mcb/mcb-lpc.c
> >> index 506676754538..53decd89876e 100644
> >> --- a/drivers/mcb/mcb-lpc.c
> >> +++ b/drivers/mcb/mcb-lpc.c
> >> @@ -105,17 +105,8 @@ static int mcb_lpc_create_platform_device(const struct dmi_system_id *id)
> >> return ret;
> >> }
> >>
> >> -static struct resource sc24_fpga_resource = {
> >> - .start = 0xe000e000,
> >> - .end = 0xe000e000 + CHAM_HEADER_SIZE,
> >> - .flags = IORESOURCE_MEM,
> >> -};
> >> -
> >> -static struct resource sc31_fpga_resource = {
> >> - .start = 0xf000e000,
> >> - .end = 0xf000e000 + CHAM_HEADER_SIZE,
> >> - .flags = IORESOURCE_MEM,
> >> -};
> >> +static struct resource sc24_fpga_resource = DEFINE_RES_MEM(0xe000e000, CHAM_HEADER_SIZE);
> >> +static struct resource sc31_fpga_resource = DEFINE_RES_MEM(0xf000e000, CHAM_HEADER_SIZE);
> >
> > Does this simplify stuff, or does it fix a bug with the existing
> > definition?
>
> It does not fix a bug, the actual value of .end should be "0xe000e000 + CHAM_HEADER_SIZE - 1".
> There is no functional problem, just a little more memory. So it's just been corrected in the
> process of simplification.
>
> Do you think it's necessary to split it into two patches?
No need, I just want to know if this is a fix that needs to go to older
kernels as well. The text was very vague.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-16 6:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 14:55 [PATCH 0/3] mcb changes for 5.14 Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 1/3] mcb: Use DEFINE_RES_MEM() to simplify code Johannes Thumshirn
2021-06-15 15:42 ` Greg Kroah-Hartman
2021-06-16 1:31 ` Leizhen (ThunderTown)
2021-06-16 6:41 ` Greg Kroah-Hartman
2021-06-15 14:55 ` [PATCH 2/3] mcb: fix error handling in mcb_alloc_bus() Johannes Thumshirn
2021-06-15 15:41 ` Greg Kroah-Hartman
2021-06-15 15:44 ` Johannes Thumshirn
2021-06-15 14:55 ` [PATCH 3/3] mcb: Remove trailing semicolon in macros Johannes Thumshirn
2021-06-15 15:48 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.