* [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
* 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 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
* [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
* 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 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
* [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 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
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.