[PATCH] net/mlx4_core: Add missed mlx4_free_cmd_mailbox()
mlx4_do_mirror_rule() forgets to call mlx4_free_cmd_mailbox() to free the memory region allocated by mlx4_alloc_cmd_mailbox() before an exit. Add the missed call to fix it. Fixes: 78efed275117 ("net/mlx4_core: Support mirroring VF DMFS rules on both ports") Signed-off-by: Chuhong Yuan --- drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c index 394f43add85c..a99e71bc7b3c 100644 --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c @@ -4986,6 +4986,7 @@ static int mlx4_do_mirror_rule(struct mlx4_dev *dev, struct res_fs_rule *fs_rule if (!fs_rule->mirr_mbox) { mlx4_err(dev, "rule mirroring mailbox is null\n"); + mlx4_free_cmd_mailbox(dev, mailbox); return -EINVAL; } memcpy(mailbox->buf, fs_rule->mirr_mbox, fs_rule->mirr_mbox_size); -- 2.27.0
[PATCH v2] ASoC: amd: change clk_get() to devm_clk_get() and add missed checks
cz_da7219_init() does not check the return values of clk_get(), while da7219_clk_enable() calls clk_set_rate() to dereference the pointers. Add checks to fix the problems. Also, change clk_get() to devm_clk_get() to avoid data leak after failures. Fixes: bb24a31ed584 ("ASoC: AMD: Configure wclk and bclk of master codec") Signed-off-by: Chuhong Yuan --- Changes in v2: - Replace clk_get() with devm_clk_get() to prevent data leak. sound/soc/amd/acp-da7219-max98357a.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c index a7702e64ec51..849288d01c6b 100644 --- a/sound/soc/amd/acp-da7219-max98357a.c +++ b/sound/soc/amd/acp-da7219-max98357a.c @@ -73,8 +73,13 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd) return ret; } - da7219_dai_wclk = clk_get(component->dev, "da7219-dai-wclk"); - da7219_dai_bclk = clk_get(component->dev, "da7219-dai-bclk"); + da7219_dai_wclk = devm_clk_get(component->dev, "da7219-dai-wclk"); + if (IS_ERR(da7219_dai_wclk)) + return PTR_ERR(da7219_dai_wclk); + + da7219_dai_bclk = devm_clk_get(component->dev, "da7219-dai-bclk"); + if (IS_ERR(da7219_dai_bclk)) + return PTR_ERR(da7219_dai_bclk); ret = snd_soc_card_jack_new(card, "Headset Jack", SND_JACK_HEADSET | SND_JACK_LINEOUT | -- 2.26.2
[PATCH] drm/fb-helper: Add missed unlocks in setcmap_legacy()
setcmap_legacy() does not call drm_modeset_unlock_all() in some exits, add the missed unlocks with goto to fix it. Fixes: 964c60063bff ("drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths") Signed-off-by: Chuhong Yuan --- drivers/gpu/drm/drm_fb_helper.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 1543d9d10970..8033467db4be 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -923,11 +923,15 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) drm_modeset_lock_all(fb_helper->dev); drm_client_for_each_modeset(modeset, &fb_helper->client) { crtc = modeset->crtc; - if (!crtc->funcs->gamma_set || !crtc->gamma_size) - return -EINVAL; + if (!crtc->funcs->gamma_set || !crtc->gamma_size) { + ret = -EINVAL; + goto out; + } - if (cmap->start + cmap->len > crtc->gamma_size) - return -EINVAL; + if (cmap->start + cmap->len > crtc->gamma_size) { + ret = -EINVAL; + goto out; + } r = crtc->gamma_store; g = r + crtc->gamma_size; @@ -940,8 +944,9 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) ret = crtc->funcs->gamma_set(crtc, r, g, b, crtc->gamma_size, NULL); if (ret) - return ret; + goto out; } +out: drm_modeset_unlock_all(fb_helper->dev); return ret; -- 2.26.2
[PATCH] ASoC: jz4740-i2s: add missed checks for clk_get()
jz4740_i2s_set_sysclk() does not check the return values of clk_get(), while the file dereferences the pointers in clk_put(). Add the missed checks to fix it. Fixes: 11bd3dd1b7c2 ("ASoC: Add JZ4740 ASoC support") Signed-off-by: Chuhong Yuan --- sound/soc/jz4740/jz4740-i2s.c | 4 1 file changed, 4 insertions(+) diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index c7bd20104b20..0793e284d0e7 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -312,10 +312,14 @@ static int jz4740_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, switch (clk_id) { case JZ4740_I2S_CLKSRC_EXT: parent = clk_get(NULL, "ext"); + if (IS_ERR(parent)) + return PTR_ERR(parent); clk_set_parent(i2s->clk_i2s, parent); break; case JZ4740_I2S_CLKSRC_PLL: parent = clk_get(NULL, "pll half"); + if (IS_ERR(parent)) + return PTR_ERR(parent); clk_set_parent(i2s->clk_i2s, parent); ret = clk_set_rate(i2s->clk_i2s, freq); break; -- 2.26.2
[PATCH] ASoC: amd: add missed checks for clk_get()
cz_da7219_init() does not check the return value of clk_get(), while da7219_clk_enable() calls clk_set_rate() to dereference the pointers. Add checks to fix the problems. Fixes: bb24a31ed584 ("ASoC: AMD: Configure wclk and bclk of master codec") Signed-off-by: Chuhong Yuan --- sound/soc/amd/acp-da7219-max98357a.c | 5 + 1 file changed, 5 insertions(+) diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c index a7702e64ec51..d5185cd3106b 100644 --- a/sound/soc/amd/acp-da7219-max98357a.c +++ b/sound/soc/amd/acp-da7219-max98357a.c @@ -74,7 +74,12 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd) } da7219_dai_wclk = clk_get(component->dev, "da7219-dai-wclk"); + if (IS_ERR(da7219_dai_wclk)) + return PTR_ERR(da7219_dai_wclk); + da7219_dai_bclk = clk_get(component->dev, "da7219-dai-bclk"); + if (IS_ERR(da7219_dai_bclk)) + return PTR_ERR(da7219_dai_bclk); ret = snd_soc_card_jack_new(card, "Headset Jack", SND_JACK_HEADSET | SND_JACK_LINEOUT | -- 2.26.2
[PATCH v2] serial: mxs-auart: add missed iounmap() in probe failure and remove
This driver calls ioremap() in probe, but it misses calling iounmap() in probe's error handler and remove. Add the missed calls to fix it. Fixes: 47d37d6f94cc ("serial: Add auart driver for i.MX23/28") Signed-off-by: Chuhong Yuan --- Changes in v2: - Use iounmap() instead of devm_ioremap() to fix the bugs. - Modify the subject and the description. drivers/tty/serial/mxs-auart.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c index b4f835e7de23..b784323a6a7b 100644 --- a/drivers/tty/serial/mxs-auart.c +++ b/drivers/tty/serial/mxs-auart.c @@ -1698,21 +1698,21 @@ static int mxs_auart_probe(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq < 0) { ret = irq; - goto out_disable_clks; + goto out_iounmap; } s->port.irq = irq; ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s); if (ret) - goto out_disable_clks; + goto out_iounmap; platform_set_drvdata(pdev, s); ret = mxs_auart_init_gpios(s, &pdev->dev); if (ret) { dev_err(&pdev->dev, "Failed to initialize GPIOs.\n"); - goto out_disable_clks; + goto out_iounmap; } /* @@ -1720,7 +1720,7 @@ static int mxs_auart_probe(struct platform_device *pdev) */ ret = mxs_auart_request_gpio_irq(s); if (ret) - goto out_disable_clks; + goto out_iounmap; auart_port[s->port.line] = s; @@ -1746,6 +1746,9 @@ static int mxs_auart_probe(struct platform_device *pdev) mxs_auart_free_gpio_irq(s); auart_port[pdev->id] = NULL; +out_iounmap: + iounmap(s->port.membase); + out_disable_clks: if (is_asm9260_auart(s)) { clk_disable_unprepare(s->clk); @@ -1761,6 +1764,7 @@ static int mxs_auart_remove(struct platform_device *pdev) uart_remove_one_port(&auart_driver, &s->port); auart_port[pdev->id] = NULL; mxs_auart_free_gpio_irq(s); + iounmap(s->port.membase); if (is_asm9260_auart(s)) { clk_disable_unprepare(s->clk); clk_disable_unprepare(s->clk_ahb); -- 2.26.2
[PATCH] USB: ohci-sm501: Add missed iounmap() in remove
This driver misses calling iounmap() in remove to undo the ioremap() called in probe. Add the missed call to fix it. Fixes: f54aab6ebcec ("usb: ohci-sm501 driver") Signed-off-by: Chuhong Yuan --- drivers/usb/host/ohci-sm501.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c index cff965240327..b91d50da6127 100644 --- a/drivers/usb/host/ohci-sm501.c +++ b/drivers/usb/host/ohci-sm501.c @@ -191,6 +191,7 @@ static int ohci_hcd_sm501_drv_remove(struct platform_device *pdev) struct resource *mem; usb_remove_hcd(hcd); + iounmap(hcd->regs); release_mem_region(hcd->rsrc_start, hcd->rsrc_len); usb_put_hcd(hcd); mem = platform_get_resource(pdev, IORESOURCE_MEM, 1); -- 2.26.2
[PATCH] mmc: sdhci-of-arasan: Add missed checks for devm_clk_register()
These functions do not check the return value of devm_clk_register(): - sdhci_arasan_register_sdcardclk() - sdhci_arasan_register_sampleclk() Therefore, add the missed checks to fix them. Fixes: c390f2110adf1 ("mmc: sdhci-of-arasan: Add ability to export card clock") Signed-off-by: Chuhong Yuan --- drivers/mmc/host/sdhci-of-arasan.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index db9b544465cd..fb26e743e1fd 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -1299,6 +1299,8 @@ sdhci_arasan_register_sdcardclk(struct sdhci_arasan_data *sdhci_arasan, clk_data->sdcardclk_hw.init = &sdcardclk_init; clk_data->sdcardclk = devm_clk_register(dev, &clk_data->sdcardclk_hw); + if (IS_ERR(clk_data->sdcardclk)) + return PTR_ERR(clk_data->sdcardclk); clk_data->sdcardclk_hw.init = NULL; ret = of_clk_add_provider(np, of_clk_src_simple_get, @@ -1349,6 +1351,8 @@ sdhci_arasan_register_sampleclk(struct sdhci_arasan_data *sdhci_arasan, clk_data->sampleclk_hw.init = &sampleclk_init; clk_data->sampleclk = devm_clk_register(dev, &clk_data->sampleclk_hw); + if (IS_ERR(clk_data->sampleclk)) + return PTR_ERR(clk_data->sampleclk); clk_data->sampleclk_hw.init = NULL; ret = of_clk_add_provider(np, of_clk_src_simple_get, -- 2.26.2
[PATCH] serial: mxs-auart: Use devm_ioremap() to fix the missing undo bug
This driver calls ioremap() in probe, but it misses calling iounmap() in probe's error handler and remove. Replace ioremap() with the devm version to fix it. Fixes: 47d37d6f94cc ("serial: Add auart driver for i.MX23/28") Signed-off-by: Chuhong Yuan --- drivers/tty/serial/mxs-auart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c index b4f835e7de23..b3e16fd72eaf 100644 --- a/drivers/tty/serial/mxs-auart.c +++ b/drivers/tty/serial/mxs-auart.c @@ -1679,7 +1679,7 @@ static int mxs_auart_probe(struct platform_device *pdev) } s->port.mapbase = r->start; - s->port.membase = ioremap(r->start, resource_size(r)); + s->port.membase = devm_ioremap(&pdev->dev, r->start, resource_size(r)); if (!s->port.membase) { ret = -ENOMEM; goto out_disable_clks; -- 2.26.2
[PATCH] mfd: tc3589x: Use devm_request_threaded_irq() to fix the missing undo bug
This driver calls request_threaded_irq() in probe, but it misses calling free_irq() in probe's error handler and remove. Replace request_threaded_irq() with the devm version to fix it. Fixes: 20406ebff4a2 ("mfd/tc3589x: rename tc35892 structs/registers to tc359x") Signed-off-by: Chuhong Yuan --- drivers/mfd/tc3589x.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/mfd/tc3589x.c b/drivers/mfd/tc3589x.c index 67c9995bb1aa..0fd8ba1c68d0 100644 --- a/drivers/mfd/tc3589x.c +++ b/drivers/mfd/tc3589x.c @@ -412,9 +412,9 @@ static int tc3589x_probe(struct i2c_client *i2c, if (ret) return ret; - ret = request_threaded_irq(tc3589x->i2c->irq, NULL, tc3589x_irq, - IRQF_TRIGGER_FALLING | IRQF_ONESHOT, - "tc3589x", tc3589x); + ret = devm_request_threaded_irq(&i2c->dev, tc3589x->i2c->irq, NULL, + tc3589x_irq, IRQF_TRIGGER_FALLING | + IRQF_ONESHOT, "tc3589x", tc3589x); if (ret) { dev_err(tc3589x->dev, "failed to request IRQ: %d\n", ret); return ret; -- 2.26.2
[PATCH] media: tvp5150: Add missed media_entity_cleanup()
This driver does not call media_entity_cleanup() in the error handler of tvp5150_registered() and tvp5150_remove(), while it has called media_entity_pads_init() at first. Add the missed calls to fix it. Fixes: 0556f1d580d4 ("media: tvp5150: add input source selection of_graph support") Signed-off-by: Chuhong Yuan --- drivers/media/i2c/tvp5150.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index eb39cf5ea089..9df575238952 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1664,8 +1664,10 @@ static int tvp5150_registered(struct v4l2_subdev *sd) return 0; err: - for (i = 0; i < decoder->connectors_num; i++) + for (i = 0; i < decoder->connectors_num; i++) { media_device_unregister_entity(&decoder->connectors[i].ent); + media_entity_cleanup(&decoder->connectors[i].ent); + } return ret; #endif @@ -2248,8 +2250,10 @@ static int tvp5150_remove(struct i2c_client *c) for (i = 0; i < decoder->connectors_num; i++) v4l2_fwnode_connector_free(&decoder->connectors[i].base); - for (i = 0; i < decoder->connectors_num; i++) + for (i = 0; i < decoder->connectors_num; i++) { media_device_unregister_entity(&decoder->connectors[i].ent); + media_entity_cleanup(&decoder->connectors[i].ent); + } v4l2_async_unregister_subdev(sd); v4l2_ctrl_handler_free(&decoder->hdl); pm_runtime_disable(&c->dev); -- 2.26.2
[PATCH v2] media: budget-core: Improve exception handling in budget_register()
budget_register() has no error handling after its failure. Add the missed undo functions for error handling to fix it. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Chuhong Yuan --- Changes in v2: - Modify the label of error handler. drivers/media/pci/ttpci/budget-core.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/media/pci/ttpci/budget-core.c b/drivers/media/pci/ttpci/budget-core.c index fadbdeeb4495..293867b9e796 100644 --- a/drivers/media/pci/ttpci/budget-core.c +++ b/drivers/media/pci/ttpci/budget-core.c @@ -369,20 +369,25 @@ static int budget_register(struct budget *budget) ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &budget->hw_frontend); if (ret < 0) - return ret; + goto err_release_dmx; budget->mem_frontend.source = DMX_MEMORY_FE; ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &budget->mem_frontend); if (ret < 0) - return ret; + goto err_release_dmx; ret = dvbdemux->dmx.connect_frontend(&dvbdemux->dmx, &budget->hw_frontend); if (ret < 0) - return ret; + goto err_release_dmx; dvb_net_init(&budget->dvb_adapter, &budget->dvb_net, &dvbdemux->dmx); return 0; + +err_release_dmx: + dvb_dmxdev_release(&budget->dmxdev); + dvb_dmx_release(&budget->demux); + return ret; } static void budget_unregister(struct budget *budget) -- 2.26.2
[PATCH v2] fbdev: geode: Add the missed pci_disable_device() in gx1fb_map_video_memory()
Although gx1fb_probe() has handled the failure of gx1fb_map_video_memory() partly, it does not call pci_disable_device() as gx1fb_map_video_memory() calls pci_enable_device(). Add the missed function call to fix the bug. Fixes: 53eed4ec8bcd ("[PATCH] fbdev: geode updates]") Signed-off-by: Chuhong Yuan --- Changes in v2: - Fix the typo in the subject. - Modify the label of error handler. - Refactor the code. drivers/video/fbdev/geode/gx1fb_core.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/video/fbdev/geode/gx1fb_core.c b/drivers/video/fbdev/geode/gx1fb_core.c index 5d34d89fb665..15645244e4d0 100644 --- a/drivers/video/fbdev/geode/gx1fb_core.c +++ b/drivers/video/fbdev/geode/gx1fb_core.c @@ -208,29 +208,39 @@ static int gx1fb_map_video_memory(struct fb_info *info, struct pci_dev *dev) ret = pci_request_region(dev, 0, "gx1fb (video)"); if (ret < 0) - return ret; + goto err_disable_device; par->vid_regs = pci_ioremap_bar(dev, 0); if (!par->vid_regs) - return -ENOMEM; + goto err_nomem; - if (!request_mem_region(gx_base + 0x8300, 0x100, "gx1fb (display controller)")) - return -EBUSY; + if (!request_mem_region(gx_base + 0x8300, 0x100, + "gx1fb (display controller)")) { + ret = -EBUSY; + goto err_disable_device; + } par->dc_regs = ioremap(gx_base + 0x8300, 0x100); if (!par->dc_regs) - return -ENOMEM; + goto err_nomem; if ((fb_len = gx1_frame_buffer_size()) < 0) - return -ENOMEM; + goto err_nomem; + info->fix.smem_start = gx_base + 0x80; info->fix.smem_len = fb_len; info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); if (!info->screen_base) - return -ENOMEM; + goto err_nomem; dev_info(&dev->dev, "%d Kibyte of video memory at 0x%lx\n", info->fix.smem_len / 1024, info->fix.smem_start); return 0; + +err_nomem: + ret = -ENOMEM; +err_disable_device: + pci_disable_device(dev); + return ret; } static int parse_panel_option(struct fb_info *info) -- 2.26.2
[PATCH v2] clk: clk-st: Add missed return value checks in st_clk_probe()
Return values were not checked after calls of the following functions. - clk_hw_register_mux - clk_hw_register_gate - devm_clk_hw_register_clkdev Thus add error detection and the corresponding exception handling. Return the value from the function call "devm_clk_hw_register_clkdev" at the end of this function implementation. Fixes: 421bf6a1f061 ("clk: x86: Add ST oscout platform clock") Signed-off-by: Chuhong Yuan --- Changes in v2: - Modify description. drivers/clk/x86/clk-st.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c index 25d4b97aff9b..e6ee6ea2568b 100644 --- a/drivers/clk/x86/clk-st.c +++ b/drivers/clk/x86/clk-st.c @@ -46,16 +46,20 @@ static int st_clk_probe(struct platform_device *pdev) clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents), 0, st_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0, NULL); + if (IS_ERR(hws[ST_CLK_MUX])) + return PTR_ERR(hws[ST_CLK_MUX]); + clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk); hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", "oscout1_mux", 0, st_data->base + MISCCLKCNTL1, OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL); - devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], "oscout1", - NULL); + if (IS_ERR(hws[ST_CLK_GATE])) + return PTR_ERR(hws[ST_CLK_GATE]); - return 0; + return devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], + "oscout1", NULL); } static int st_clk_remove(struct platform_device *pdev) -- 2.26.2
[PATCH v2] media: omap3isp: Add missed v4l2_ctrl_handler_free() for preview_init_entities()
preview_init_entities() does not call v4l2_ctrl_handler_free() when it fails. Add the missed function to fix it. Fixes: de1135d44f4f ("[media] omap3isp: CCDC, preview engine and resizer]") Signed-off-by: Chuhong Yuan --- Changes in v2: - Fix the typo. drivers/media/platform/omap3isp/isppreview.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c index 4dbdf3180d10..607b7685c982 100644 --- a/drivers/media/platform/omap3isp/isppreview.c +++ b/drivers/media/platform/omap3isp/isppreview.c @@ -2287,7 +2287,7 @@ static int preview_init_entities(struct isp_prev_device *prev) me->ops = &preview_media_ops; ret = media_entity_pads_init(me, PREV_PADS_NUM, pads); if (ret < 0) - return ret; + goto error_handler_free; preview_init_formats(sd, NULL); @@ -2320,6 +2320,8 @@ static int preview_init_entities(struct isp_prev_device *prev) omap3isp_video_cleanup(&prev->video_in); error_video_in: media_entity_cleanup(&prev->subdev.entity); +error_handler_free: + v4l2_ctrl_handler_free(&prev->ctrls); return ret; } -- 2.26.2
[PATCH v2] media: marvell-ccic: Add missed v4l2_async_notifier_cleanup()
mccic_register() forgets to cleanup the notifier in its error handler. mccic_shutdown() also misses calling v4l2_async_notifier_cleanup(). Add the missed calls to fix them. Fixes: 3eefe36cc00c ("media: marvell-ccic: use async notifier to get the sensor") Signed-off-by: Chuhong Yuan --- Changes in v2: - Also add v4l2_async_notifier_cleanup() for mccic_shutdown(). - Modify description. drivers/media/platform/marvell-ccic/mcam-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 09775b6624c6..326e79b8531c 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -1940,6 +1940,7 @@ int mccic_register(struct mcam_camera *cam) out: v4l2_async_notifier_unregister(&cam->notifier); v4l2_device_unregister(&cam->v4l2_dev); + v4l2_async_notifier_cleanup(&cam->notifier); return ret; } EXPORT_SYMBOL_GPL(mccic_register); @@ -1961,6 +1962,7 @@ void mccic_shutdown(struct mcam_camera *cam) v4l2_ctrl_handler_free(&cam->ctrl_handler); v4l2_async_notifier_unregister(&cam->notifier); v4l2_device_unregister(&cam->v4l2_dev); + v4l2_async_notifier_cleanup(&cam->notifier); } EXPORT_SYMBOL_GPL(mccic_shutdown); -- 2.26.2
[PATCH] media: omap3isp: Add missed v4l2_ctrl_handler_free() for preview_init_entities()
preview_init_entities() does not call v4l2_ctrl_handler_free() when it fails. Add the missed function to fix it. Fixes: de1135d44f4f ("[media] omap3isp: CCDC, preview engine and resizer]") Signed-off-by: Chuhong Yuan --- drivers/media/platform/omap3isp/isppreview.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c index 4dbdf3180d10..38b93ec60536 100644 --- a/drivers/media/platform/omap3isp/isppreview.c +++ b/drivers/media/platform/omap3isp/isppreview.c @@ -2287,7 +2287,7 @@ static int preview_init_entities(struct isp_prev_device *prev) me->ops = &preview_media_ops; ret = media_entity_pads_init(me, PREV_PADS_NUM, pads); if (ret < 0) - return ret; + goto err_handler_free; preview_init_formats(sd, NULL); @@ -2320,6 +2320,8 @@ static int preview_init_entities(struct isp_prev_device *prev) omap3isp_video_cleanup(&prev->video_in); error_video_in: media_entity_cleanup(&prev->subdev.entity); +error_handler_free: + v4l2_ctrl_handler_free(&prev->ctrls); return ret; } -- 2.26.2
[PATCH] media: marvell-ccic: Add missed v4l2_async_notifier_cleanup()
mccic_register() forgets to cleanup the notifier in its error handler. Add the missed call to fix it. Fixes: 3eefe36cc00c ("media: marvell-ccic: use async notifier to get the sensor") Signed-off-by: Chuhong Yuan --- drivers/media/platform/marvell-ccic/mcam-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 09775b6624c6..cf2a0119e679 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -1940,6 +1940,7 @@ int mccic_register(struct mcam_camera *cam) out: v4l2_async_notifier_unregister(&cam->notifier); v4l2_device_unregister(&cam->v4l2_dev); + v4l2_async_notifier_cleanup(&cam->notifier); return ret; } EXPORT_SYMBOL_GPL(mccic_register); -- 2.26.2
[PATCH] xfs: Add the missed xfs_perag_put() for xfs_ifree_cluster()
xfs_ifree_cluster() calls xfs_perag_get() at the beginning, but forgets to call xfs_perag_put() in one failed path. Add the missed function call to fix it. Fixes: ce92464c180b ("xfs: make xfs_trans_get_buf return an error code") Signed-off-by: Chuhong Yuan --- fs/xfs/xfs_inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index d1772786af29..8845faa8161a 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2639,8 +2639,10 @@ xfs_ifree_cluster( error = xfs_trans_get_buf(tp, mp->m_ddev_targp, blkno, mp->m_bsize * igeo->blocks_per_cluster, XBF_UNMAPPED, &bp); - if (error) + if (error) { + xfs_perag_put(pag); return error; + } /* * This buffer may not have been correctly initialised as we -- 2.26.2
[PATCH] media: budget-core: Add missed undo functions in budget_register()
budget_register() has no error handling after its failure. Add the missed undo functions for error handling to fix it. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Chuhong Yuan --- drivers/media/pci/ttpci/budget-core.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/media/pci/ttpci/budget-core.c b/drivers/media/pci/ttpci/budget-core.c index fadbdeeb4495..53aa34a3160b 100644 --- a/drivers/media/pci/ttpci/budget-core.c +++ b/drivers/media/pci/ttpci/budget-core.c @@ -369,20 +369,25 @@ static int budget_register(struct budget *budget) ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &budget->hw_frontend); if (ret < 0) - return ret; + goto err; budget->mem_frontend.source = DMX_MEMORY_FE; ret = dvbdemux->dmx.add_frontend(&dvbdemux->dmx, &budget->mem_frontend); if (ret < 0) - return ret; + goto err; ret = dvbdemux->dmx.connect_frontend(&dvbdemux->dmx, &budget->hw_frontend); if (ret < 0) - return ret; + goto err; dvb_net_init(&budget->dvb_adapter, &budget->dvb_net, &dvbdemux->dmx); return 0; + +err: + dvb_dmxdev_release(&budget->dmxdev); + dvb_dmx_release(&budget->demux); + return ret; } static void budget_unregister(struct budget *budget) -- 2.26.2
[PATCH v3] iio: amplifiers: ad8366: Change devm_gpiod_get() to optional and add the missed check
Since if there is no GPIO, nothing happens, replace devm_gpiod_get() with devm_gpiod_get_optional(). Also add IS_ERR() to fix the missing-check bug. Fixes: cee211f4e5a0 ("iio: amplifiers: ad8366: Add support for the ADA4961 DGA") Signed-off-by: Chuhong Yuan --- Changes in v3: - Change devm_gpiod_get() to optional. - Modify description. drivers/iio/amplifiers/ad8366.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c index 62167b87caea..8819e8997f76 100644 --- a/drivers/iio/amplifiers/ad8366.c +++ b/drivers/iio/amplifiers/ad8366.c @@ -262,8 +262,12 @@ static int ad8366_probe(struct spi_device *spi) case ID_ADA4961: case ID_ADL5240: case ID_HMC1119: - st->reset_gpio = devm_gpiod_get(&spi->dev, "reset", + st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(st->reset_gpio)) { + ret = PTR_ERR(st->reset_gpio); + goto error_disable_reg; + } indio_dev->channels = ada4961_channels; indio_dev->num_channels = ARRAY_SIZE(ada4961_channels); break; -- 2.26.2
[PATCH] fbdev: geocode: Add the missed pci_disable_device() for gx1fb_map_video_memory()
Although gx1fb_probe() has handled the failure of gx1fb_map_video_memory() partly, it does not call pci_disable_device() as gx1fb_map_video_memory() calls pci_enable_device(). Add the missed function call to fix the bug. Fixes: 53eed4ec8bcd ("[PATCH] fbdev: geode updates]") Signed-off-by: Chuhong Yuan --- drivers/video/fbdev/geode/gx1fb_core.c | 37 ++ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/video/fbdev/geode/gx1fb_core.c b/drivers/video/fbdev/geode/gx1fb_core.c index 5d34d89fb665..c9465542204a 100644 --- a/drivers/video/fbdev/geode/gx1fb_core.c +++ b/drivers/video/fbdev/geode/gx1fb_core.c @@ -208,29 +208,44 @@ static int gx1fb_map_video_memory(struct fb_info *info, struct pci_dev *dev) ret = pci_request_region(dev, 0, "gx1fb (video)"); if (ret < 0) - return ret; + goto err; par->vid_regs = pci_ioremap_bar(dev, 0); - if (!par->vid_regs) - return -ENOMEM; + if (!par->vid_regs) { + ret = -ENOMEM; + goto err; + } - if (!request_mem_region(gx_base + 0x8300, 0x100, "gx1fb (display controller)")) - return -EBUSY; + if (!request_mem_region(gx_base + 0x8300, 0x100, + "gx1fb (display controller)")) { + ret = -EBUSY; + goto err; + } par->dc_regs = ioremap(gx_base + 0x8300, 0x100); - if (!par->dc_regs) - return -ENOMEM; + if (!par->dc_regs) { + ret = -ENOMEM; + goto err; + } - if ((fb_len = gx1_frame_buffer_size()) < 0) - return -ENOMEM; + if ((fb_len = gx1_frame_buffer_size()) < 0) { + ret = -ENOMEM; + goto err; + } info->fix.smem_start = gx_base + 0x80; info->fix.smem_len = fb_len; info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len); - if (!info->screen_base) - return -ENOMEM; + if (!info->screen_base) { + ret = -ENOMEM; + goto err; + } dev_info(&dev->dev, "%d Kibyte of video memory at 0x%lx\n", info->fix.smem_len / 1024, info->fix.smem_start); return 0; + +err: + pci_disable_device(dev); + return ret; } static int parse_panel_option(struct fb_info *info) -- 2.26.2
[PATCH] ALSA: es1688: Add the missed snd_card_free()
snd_es968_pnp_detect() misses a snd_card_free() in a failed path. Add the missed function call to fix it. Fixes: a20971b201ac ("ALSA: Merge es1688 and es968 drivers") Signed-off-by: Chuhong Yuan --- sound/isa/es1688/es1688.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c index ff3a05ad99c0..64610571a5e1 100644 --- a/sound/isa/es1688/es1688.c +++ b/sound/isa/es1688/es1688.c @@ -267,8 +267,10 @@ static int snd_es968_pnp_detect(struct pnp_card_link *pcard, return error; } error = snd_es1688_probe(card, dev); - if (error < 0) + if (error < 0) { + snd_card_free(card); return error; + } pnp_set_card_drvdata(pcard, card); snd_es968_pnp_is_probed = 1; return 0; -- 2.26.2
[PATCH] clk: clk-st: Add the missed checks in st_clk_probe()
st_clk_probe() has not check for clk_hw_register_mux(), clk_hw_register_gate() and devm_clk_hw_register_clkdev(). Add the missed checks and return devm_clk_hw_register_clkdev()'s return value to check errors. Fixes: 421bf6a1f061 ("clk: x86: Add ST oscout platform clock") Signed-off-by: Chuhong Yuan --- drivers/clk/x86/clk-st.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c index 25d4b97aff9b..e6ee6ea2568b 100644 --- a/drivers/clk/x86/clk-st.c +++ b/drivers/clk/x86/clk-st.c @@ -46,16 +46,20 @@ static int st_clk_probe(struct platform_device *pdev) clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents), 0, st_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0, NULL); + if (IS_ERR(hws[ST_CLK_MUX])) + return PTR_ERR(hws[ST_CLK_MUX]); + clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk); hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", "oscout1_mux", 0, st_data->base + MISCCLKCNTL1, OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL); - devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], "oscout1", - NULL); + if (IS_ERR(hws[ST_CLK_GATE])) + return PTR_ERR(hws[ST_CLK_GATE]); - return 0; + return devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], + "oscout1", NULL); } static int st_clk_remove(struct platform_device *pdev) -- 2.26.2
[PATCH v2] Bluetooth: btmtkuart: Improve exception handling in btmtuart_probe()
Calls of the functions clk_disable_unprepare() and hci_free_dev() were missing for the exception handling. Thus add the missed function calls together with corresponding jump targets. Fixes: 055825614c6b ("Bluetooth: btmtkuart: add an implementation for clock osc property") Signed-off-by: Chuhong Yuan --- Changes in v2: - Modify description. - Add fixes tag. drivers/bluetooth/btmtkuart.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c index e11169ad8247..8a81fbca5c9d 100644 --- a/drivers/bluetooth/btmtkuart.c +++ b/drivers/bluetooth/btmtkuart.c @@ -1015,7 +1015,7 @@ static int btmtkuart_probe(struct serdev_device *serdev) if (btmtkuart_is_standalone(bdev)) { err = clk_prepare_enable(bdev->osc); if (err < 0) - return err; + goto err_hci_free_dev; if (bdev->boot) { gpiod_set_value_cansleep(bdev->boot, 1); @@ -1028,10 +1028,8 @@ static int btmtkuart_probe(struct serdev_device *serdev) /* Power on */ err = regulator_enable(bdev->vcc); - if (err < 0) { - clk_disable_unprepare(bdev->osc); - return err; - } + if (err < 0) + goto err_clk_disable_unprepare; /* Reset if the reset-gpios is available otherwise the board * -level design should be guaranteed. @@ -1063,7 +1061,6 @@ static int btmtkuart_probe(struct serdev_device *serdev) err = hci_register_dev(hdev); if (err < 0) { dev_err(&serdev->dev, "Can't register HCI device\n"); - hci_free_dev(hdev); goto err_regulator_disable; } @@ -1072,6 +1069,11 @@ static int btmtkuart_probe(struct serdev_device *serdev) err_regulator_disable: if (btmtkuart_is_standalone(bdev)) regulator_disable(bdev->vcc); +err_clk_disable_unprepare: + if (btmtkuart_is_standalone(bdev)) + clk_disable_unprepare(bdev->osc); +err_hci_free_dev: + hci_free_dev(hdev); return err; } -- 2.26.2
[PATCH v2] rtc: rv3028: Add missed check for devm_regmap_init_i2c()
rv3028_probe() misses a check for devm_regmap_init_i2c(). Add the missed check to fix it. Fixes: e6e7376cfd7b ("rtc: rv3028: add new driver") Signed-off-by: Chuhong Yuan --- Changes in v2: - Add fixes tag. drivers/rtc/rtc-rv3028.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c index a0ddc86c975a..ec84db0b3d7a 100644 --- a/drivers/rtc/rtc-rv3028.c +++ b/drivers/rtc/rtc-rv3028.c @@ -755,6 +755,8 @@ static int rv3028_probe(struct i2c_client *client) return -ENOMEM; rv3028->regmap = devm_regmap_init_i2c(client, ®map_config); + if (IS_ERR(rv3028->regmap)) + return PTR_ERR(rv3028->regmap); i2c_set_clientdata(client, rv3028); -- 2.26.2
[PATCH] sbp-target: add the missed kfree() in an error path
sbp_fetch_command() forgets to call kfree() in an error path. Add the missed call to fix it. Fixes: a511ce339780 ("sbp-target: Initial merge of firewire/ieee-1394 target mode support") Signed-off-by: Chuhong Yuan --- drivers/target/sbp/sbp_target.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c index e4a9b9fe3dfb..504a755ea344 100644 --- a/drivers/target/sbp/sbp_target.c +++ b/drivers/target/sbp/sbp_target.c @@ -1128,8 +1128,10 @@ static int sbp_fetch_command(struct sbp_target_request *req) req->orb_pointer + sizeof(req->orb), req->cmd_buf + sizeof(req->orb.command_block), copy_len); - if (ret != RCODE_COMPLETE) + if (ret != RCODE_COMPLETE) { + kfree(req->cmd_buf); return -EIO; + } } return 0; -- 2.26.2
Re: [PATCH] rtc: rv3028: Add missed check for devm_regmap_init_i2c()
On Thu, May 28, 2020 at 5:17 PM Markus Elfring wrote: > > > rv3028_probe() misses a check for devm_regmap_init_i2c(). > > Such information is helpful. > > > > Add the missed check > > Thanks for another completion of the exception handling. > > > > to fix it. > > I suggest to replace this wording by the tag “Fixes” for the commit message. > How do you think about to take this possibility into account > for any more patches? > I got it. I have added fixes tags in my newly sent patches. Regards, Chuhong > Regards, > Markus
[PATCH] NFC: st21nfca: add missed kfree_skb() in an error path
st21nfca_tm_send_atr_res() misses to call kfree_skb() in an error path. Add the missed function call to fix it. Fixes: 1892bf844ea0 ("NFC: st21nfca: Adding P2P support to st21nfca in Initiator & Target mode") Signed-off-by: Chuhong Yuan --- drivers/nfc/st21nfca/dep.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nfc/st21nfca/dep.c b/drivers/nfc/st21nfca/dep.c index a1d69f9b2d4a..0b9ca6d20ffa 100644 --- a/drivers/nfc/st21nfca/dep.c +++ b/drivers/nfc/st21nfca/dep.c @@ -173,8 +173,10 @@ static int st21nfca_tm_send_atr_res(struct nfc_hci_dev *hdev, memcpy(atr_res->gbi, atr_req->gbi, gb_len); r = nfc_set_remote_general_bytes(hdev->ndev, atr_res->gbi, gb_len); - if (r < 0) + if (r < 0) { + kfree_skb(skb); return r; + } } info->dep_info.curr_nfc_dep_pni = 0; -- 2.26.2
[PATCH] Bluetooth: btmtkuart: add missed functions in the error paths of btmtuart_probe()
btmtuart_probe() misses several function calls in its error paths, including hci_free_dev() and clk_disable_unprepare(). Refactor the code and call correct undo functions to fix the error paths. Signed-off-by: Chuhong Yuan --- drivers/bluetooth/btmtkuart.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c index e11169ad8247..8a81fbca5c9d 100644 --- a/drivers/bluetooth/btmtkuart.c +++ b/drivers/bluetooth/btmtkuart.c @@ -1015,7 +1015,7 @@ static int btmtkuart_probe(struct serdev_device *serdev) if (btmtkuart_is_standalone(bdev)) { err = clk_prepare_enable(bdev->osc); if (err < 0) - return err; + goto err_hci_free_dev; if (bdev->boot) { gpiod_set_value_cansleep(bdev->boot, 1); @@ -1028,10 +1028,8 @@ static int btmtkuart_probe(struct serdev_device *serdev) /* Power on */ err = regulator_enable(bdev->vcc); - if (err < 0) { - clk_disable_unprepare(bdev->osc); - return err; - } + if (err < 0) + goto err_clk_disable_unprepare; /* Reset if the reset-gpios is available otherwise the board * -level design should be guaranteed. @@ -1063,7 +1061,6 @@ static int btmtkuart_probe(struct serdev_device *serdev) err = hci_register_dev(hdev); if (err < 0) { dev_err(&serdev->dev, "Can't register HCI device\n"); - hci_free_dev(hdev); goto err_regulator_disable; } @@ -1072,6 +1069,11 @@ static int btmtkuart_probe(struct serdev_device *serdev) err_regulator_disable: if (btmtkuart_is_standalone(bdev)) regulator_disable(bdev->vcc); +err_clk_disable_unprepare: + if (btmtkuart_is_standalone(bdev)) + clk_disable_unprepare(bdev->osc); +err_hci_free_dev: + hci_free_dev(hdev); return err; } -- 2.26.2
[PATCH] ASoC: sta32x: add missed function calls in error paths
sta32x_probe() forgets to call undo functions when it fails, add the missed function calls to fix it. Signed-off-by: Chuhong Yuan --- sound/soc/codecs/sta32x.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/sta32x.c b/sound/soc/codecs/sta32x.c index db4b3ec55311..e9ccebbc31e4 100644 --- a/sound/soc/codecs/sta32x.c +++ b/sound/soc/codecs/sta32x.c @@ -893,13 +893,13 @@ static int sta32x_probe(struct snd_soc_component *component) sta32x->supplies); if (ret != 0) { dev_err(component->dev, "Failed to enable supplies: %d\n", ret); - return ret; + goto err_clk_disable_unprepare; } ret = sta32x_startup_sequence(sta32x); if (ret < 0) { dev_err(component->dev, "Failed to startup device\n"); - return ret; + goto err_regulator_bulk_disable; } /* CONFA */ @@ -983,6 +983,13 @@ static int sta32x_probe(struct snd_soc_component *component) regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); return 0; + +err_regulator_bulk_disable: + regulator_bulk_disable(ARRAY_SIZE(sta32x->supplies), sta32x->supplies); +err_clk_disable_unprepare: + if (sta32x->xti_clk) + clk_disable_unprepare(sta32x->xti_clk); + return ret; } static void sta32x_remove(struct snd_soc_component *component) -- 2.26.2
[PATCH v2] media: exynos4-is: Add missed check for pinctrl_lookup_state()
fimc_md_get_pinctrl() misses a check for pinctrl_lookup_state(). Add the missed check to fix it. Fixes: 4163851f7b99 ("[media] s5p-fimc: Use pinctrl API for camera ports configuration]") Signed-off-by: Chuhong Yuan --- Changes in v2: - Add fixes tag. drivers/media/platform/exynos4-is/media-dev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index 9aaf3b8060d5..9c31d950cddf 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -1270,6 +1270,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd) pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl, PINCTRL_STATE_IDLE); + if (IS_ERR(pctl->state_idle)) + return PTR_ERR(pctl->state_idle); + return 0; } -- 2.26.2
[PATCH v2] iio: mma8452: Add missed iio_device_unregister() call in mma8452_probe()
The function iio_device_register() was called in mma8452_probe(). But the function iio_device_unregister() was not called after a call of the function mma8452_set_freefall_mode() failed. Thus add the missed function call for one error case. Fixes: 1a965d405fc6 ("drivers:iio:accel:mma8452: added cleanup provision in case of failure.") Signed-off-by: Chuhong Yuan --- Changes in v2: - Add fixes tag. - Modify description. drivers/iio/accel/mma8452.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index 00e100fc845a..813bca7cfc3e 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -1685,10 +1685,13 @@ static int mma8452_probe(struct i2c_client *client, ret = mma8452_set_freefall_mode(data, false); if (ret < 0) - goto buffer_cleanup; + goto unregister_device; return 0; +unregister_device: + iio_device_unregister(indio_dev); + buffer_cleanup: iio_triggered_buffer_cleanup(indio_dev); -- 2.26.2
[PATCH v2] iio: amplifiers: ad8366: Add the missed check for devm_gpiod_get()
ad8366_probe() forgets to check the return value of devm_gpiod_get(). Add the missed check to fix it. Fixes: cee211f4e5a0 ("iio: amplifiers: ad8366: Add support for the ADA4961 DGA") Signed-off-by: Chuhong Yuan --- Changes in v2: - Add fixes tag. drivers/iio/amplifiers/ad8366.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c index 62167b87caea..b996823c8d51 100644 --- a/drivers/iio/amplifiers/ad8366.c +++ b/drivers/iio/amplifiers/ad8366.c @@ -264,6 +264,10 @@ static int ad8366_probe(struct spi_device *spi) case ID_HMC1119: st->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(st->reset_gpio)) { + ret = PTR_ERR(st->reset_gpio); + goto error_disable_reg; + } indio_dev->channels = ada4961_channels; indio_dev->num_channels = ARRAY_SIZE(ada4961_channels); break; -- 2.26.2
[PATCH] rtc: rv3028: add the missed check for devm_regmap_init_i2c
rv3028_probe() misses a check for devm_regmap_init_i2c(). Add the missed check to fix it. Signed-off-by: Chuhong Yuan --- drivers/rtc/rtc-rv3028.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c index a0ddc86c975a..ec84db0b3d7a 100644 --- a/drivers/rtc/rtc-rv3028.c +++ b/drivers/rtc/rtc-rv3028.c @@ -755,6 +755,8 @@ static int rv3028_probe(struct i2c_client *client) return -ENOMEM; rv3028->regmap = devm_regmap_init_i2c(client, ®map_config); + if (IS_ERR(rv3028->regmap)) + return PTR_ERR(rv3028->regmap); i2c_set_clientdata(client, rv3028); -- 2.26.2
[PATCH] media: exynos4-is: add the missed check for pinctrl_lookup_state
From: Chuhong Yuan fimc_md_get_pinctrl() misses a check for pinctrl_lookup_state(). Add the missed check to fix it. Signed-off-by: Chuhong Yuan --- drivers/media/platform/exynos4-is/media-dev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index 9aaf3b8060d5..9c31d950cddf 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -1270,6 +1270,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd) pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl, PINCTRL_STATE_IDLE); + if (IS_ERR(pctl->state_idle)) + return PTR_ERR(pctl->state_idle); + return 0; } -- 2.26.2
[PATCH] iio: mma8452: add missed iio_device_unregister in probe failure
mma8452_probe() calls iio_device_register() but misses to call iio_device_unregister() when probe fails. Add the missed call in error handler to fix it. Signed-off-by: Chuhong Yuan --- drivers/iio/accel/mma8452.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index 00e100fc845a..813bca7cfc3e 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -1685,10 +1685,13 @@ static int mma8452_probe(struct i2c_client *client, ret = mma8452_set_freefall_mode(data, false); if (ret < 0) - goto buffer_cleanup; + goto unregister_device; return 0; +unregister_device: + iio_device_unregister(indio_dev); + buffer_cleanup: iio_triggered_buffer_cleanup(indio_dev); -- 2.26.2
[PATCH] iio: amplifiers: ad8366: add the missed check for devm_gpiod_get
ad8366_probe() forgets to check the return value of devm_gpiod_get(). Add the missed check to fix it. Signed-off-by: Chuhong Yuan --- drivers/iio/amplifiers/ad8366.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c index 62167b87caea..b996823c8d51 100644 --- a/drivers/iio/amplifiers/ad8366.c +++ b/drivers/iio/amplifiers/ad8366.c @@ -264,6 +264,10 @@ static int ad8366_probe(struct spi_device *spi) case ID_HMC1119: st->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(st->reset_gpio)) { + ret = PTR_ERR(st->reset_gpio); + goto error_disable_reg; + } indio_dev->channels = ada4961_channels; indio_dev->num_channels = ARRAY_SIZE(ada4961_channels); break; -- 2.26.2
[PATCH] uio_hv_generic: add missed sysfs_remove_bin_file
This driver calls sysfs_create_bin_file() in probe, but forgets to call sysfs_remove_bin_file() in remove. Add the missed call to fix it. Signed-off-by: Chuhong Yuan --- drivers/uio/uio_hv_generic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c index 3c5169eb23f5..4dae2320b103 100644 --- a/drivers/uio/uio_hv_generic.c +++ b/drivers/uio/uio_hv_generic.c @@ -361,6 +361,7 @@ hv_uio_remove(struct hv_device *dev) if (!pdata) return 0; + sysfs_remove_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr); uio_unregister_device(&pdata->info); hv_uio_cleanup(dev, pdata); hv_set_drvdata(dev, NULL); -- 2.26.2
[PATCH] Input: wm831x-ts - add missed input_unregister_device
This driver calls input_register_device() in probe, but misses input_unregister_device() in remove. Add the missed function call to fix it. Signed-off-by: Chuhong Yuan --- drivers/input/touchscreen/wm831x-ts.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c index 607d1aeb595d..db09dd473ada 100644 --- a/drivers/input/touchscreen/wm831x-ts.c +++ b/drivers/input/touchscreen/wm831x-ts.c @@ -379,6 +379,7 @@ static int wm831x_ts_remove(struct platform_device *pdev) { struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev); + input_unregister_device(wm831x_ts->input_dev); free_irq(wm831x_ts->pd_irq, wm831x_ts); free_irq(wm831x_ts->data_irq, wm831x_ts); -- 2.26.2
[PATCH] net: microchip: encx24j600: add missed kthread_stop
This driver calls kthread_run() in probe, but forgets to call kthread_stop() in probe failure and remove. Add the missed kthread_stop() to fix it. Signed-off-by: Chuhong Yuan --- drivers/net/ethernet/microchip/encx24j600.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/microchip/encx24j600.c b/drivers/net/ethernet/microchip/encx24j600.c index 39925e4bf2ec..b25a13da900a 100644 --- a/drivers/net/ethernet/microchip/encx24j600.c +++ b/drivers/net/ethernet/microchip/encx24j600.c @@ -1070,7 +1070,7 @@ static int encx24j600_spi_probe(struct spi_device *spi) if (unlikely(ret)) { netif_err(priv, probe, ndev, "Error %d initializing card encx24j600 card\n", ret); - goto out_free; + goto out_stop; } eidled = encx24j600_read_reg(priv, EIDLED); @@ -1088,6 +1088,8 @@ static int encx24j600_spi_probe(struct spi_device *spi) out_unregister: unregister_netdev(priv->ndev); +out_stop: + kthread_stop(priv->kworker_task); out_free: free_netdev(ndev); @@ -1100,6 +1102,7 @@ static int encx24j600_spi_remove(struct spi_device *spi) struct encx24j600_priv *priv = dev_get_drvdata(&spi->dev); unregister_netdev(priv->ndev); + kthread_stop(priv->kworker_task); free_netdev(priv->ndev); -- 2.26.2
[PATCH] Input: stmpe-ts - add missed input_unregister_device
This driver calls input_register_device() in probe, but misses input_unregister_device() in remove. Add the missed function call to fix it. Signed-off-by: Chuhong Yuan --- drivers/input/touchscreen/stmpe-ts.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c index 7e16fcfe3b95..5e62b466629b 100644 --- a/drivers/input/touchscreen/stmpe-ts.c +++ b/drivers/input/touchscreen/stmpe-ts.c @@ -350,6 +350,7 @@ static int stmpe_ts_remove(struct platform_device *pdev) { struct stmpe_touch *ts = platform_get_drvdata(pdev); + input_unregister_device(ts->idev); stmpe_disable(ts->stmpe, STMPE_BLOCK_TOUCHSCREEN); return 0; -- 2.26.2
[PATCH] Input: s6sy761 - add missed input_unregister_device
This driver calls input_register_device() in probe, but misses input_unregister_device() in probe failure and remove. Add the missed function calls to fix it. Signed-off-by: Chuhong Yuan --- drivers/input/touchscreen/s6sy761.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c index b63d7fdf0cd2..cd8a7bd9be1e 100644 --- a/drivers/input/touchscreen/s6sy761.c +++ b/drivers/input/touchscreen/s6sy761.c @@ -464,21 +464,28 @@ static int s6sy761_probe(struct i2c_client *client, IRQF_TRIGGER_LOW | IRQF_ONESHOT, "s6sy761_irq", sdata); if (err) - return err; + goto err_unregister; err = devm_device_add_group(&client->dev, &s6sy761_attribute_group); if (err) - return err; + goto err_unregister; pm_runtime_enable(&client->dev); return 0; + +err_unregister: + input_unregister_device(sdata->input); + return err; } static int s6sy761_remove(struct i2c_client *client) { + struct s6sy761_data *sdata = i2c_get_clientdata(client); + pm_runtime_disable(&client->dev); + input_unregister_device(sdata->input); return 0; } -- 2.26.2
[PATCH] Input: stmfts - add missed input_unregister_device
This driver calls input_register_device() in probe, but misses input_unregister_device() in probe failure and remove. Add the missed function calls to fix it. Signed-off-by: Chuhong Yuan --- drivers/input/touchscreen/stmfts.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c index b6f95f20f924..4345aa98a320 100644 --- a/drivers/input/touchscreen/stmfts.c +++ b/drivers/input/touchscreen/stmfts.c @@ -728,8 +728,10 @@ static int stmfts_probe(struct i2c_client *client, } err = devm_device_add_group(&client->dev, &stmfts_attribute_group); - if (err) + if (err) { + input_unregister_device(sdata->input); return err; + } pm_runtime_enable(&client->dev); device_enable_async_suspend(&client->dev); @@ -739,7 +741,10 @@ static int stmfts_probe(struct i2c_client *client, static int stmfts_remove(struct i2c_client *client) { + struct stmfts_data *sdata = i2c_get_clientdata(client); + pm_runtime_disable(&client->dev); + input_unregister_device(sdata->input); return 0; } -- 2.26.2
[PATCH] Input: applespi - add missed input_unregister_device
This driver calls input_register_device() in probe, but misses input_unregister_device() in probe failure and remove. Add the missed function calls to fix it. Signed-off-by: Chuhong Yuan --- drivers/input/keyboard/applespi.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c index d38398526965..8ee9fdb562a5 100644 --- a/drivers/input/keyboard/applespi.c +++ b/drivers/input/keyboard/applespi.c @@ -1745,7 +1745,7 @@ static int applespi_probe(struct spi_device *spi) dev_err(&applespi->spi->dev, "Failed to obtain GPE for SPI slave device: %s\n", acpi_format_exception(acpi_sts)); - return -ENODEV; + goto err_unregister; } applespi->gpe = (int)gpe; @@ -1756,7 +1756,7 @@ static int applespi_probe(struct spi_device *spi) dev_err(&applespi->spi->dev, "Failed to install GPE handler for GPE %d: %s\n", applespi->gpe, acpi_format_exception(acpi_sts)); - return -ENODEV; + goto err_unregister; } applespi->suspended = false; @@ -1767,7 +1767,7 @@ static int applespi_probe(struct spi_device *spi) "Failed to enable GPE handler for GPE %d: %s\n", applespi->gpe, acpi_format_exception(acpi_sts)); acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify); - return -ENODEV; + goto err_unregister; } /* trigger touchpad setup */ @@ -1805,6 +1805,10 @@ static int applespi_probe(struct spi_device *spi) &applespi_tp_dim_fops); return 0; + +err_unregister: + input_unregister_device(applespi->keyboard_input_dev); + return -ENODEV; } static void applespi_drain_writes(struct applespi_data *applespi) @@ -1847,6 +1851,7 @@ static int applespi_remove(struct spi_device *spi) applespi_drain_reads(applespi); debugfs_remove_recursive(applespi->debugfs_root); + input_unregister_device(applespi->keyboard_input_dev); return 0; } -- 2.26.2
[PATCH] mmc: renesas_sdhi: add checks for pinctrl_lookup_state
renesas_sdhi_probe misses checks for pinctrl_lookup_state and may miss failures. Add checks for them to fix the problem. Signed-off-by: Chuhong Yuan --- drivers/mmc/host/renesas_sdhi_core.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index d4ada5cca2d1..dc5ad6632df3 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -694,8 +694,13 @@ int renesas_sdhi_probe(struct platform_device *pdev, if (!IS_ERR(priv->pinctrl)) { priv->pins_default = pinctrl_lookup_state(priv->pinctrl, PINCTRL_STATE_DEFAULT); + if (IS_ERR(priv->pins_default)) + return PTR_ERR(priv->pins_default); + priv->pins_uhs = pinctrl_lookup_state(priv->pinctrl, "state_uhs"); + if (IS_ERR(priv->pins_uhs)) + return PTR_ERR(priv->pins_uhs); } host = tmio_mmc_host_alloc(pdev, mmc_data); -- 2.20.1
Re: [PATCH] spi: pxa2xx: Add missed security checks
On Fri, Oct 18, 2019 at 7:14 PM Andy Shevchenko wrote: > > On Fri, Oct 18, 2019 at 1:39 PM Chuhong Yuan wrote: > > > > On Fri, Oct 18, 2019 at 5:35 PM Andy Shevchenko > > wrote: > > > > > > On Fri, Oct 18, 2019 at 8:59 AM Chuhong Yuan wrote: > > > > > > > > pxa2xx_spi_init_pdata misses checks for devm_clk_get and > > > > platform_get_irq. > > > > Add checks for them to fix the bugs. > > > > > > > > Signed-off-by: Chuhong Yuan > > > > --- > > > > drivers/spi/spi-pxa2xx.c | 6 ++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > > > > index bb6a14d1ab0f..2e73d75a6ac5 100644 > > > > --- a/drivers/spi/spi-pxa2xx.c > > > > +++ b/drivers/spi/spi-pxa2xx.c > > > > @@ -1565,7 +1565,13 @@ pxa2xx_spi_init_pdata(struct platform_device > > > > *pdev) > > > > #endif > > > > > > > > ssp->clk = devm_clk_get(&pdev->dev, NULL); > > > > + if (IS_ERR(ssp->clk)) > > > > + return NULL; > > > > + > > > > ssp->irq = platform_get_irq(pdev, 0); > > > > + if (ssp->irq < 0) > > > > + return NULL; > > > > > > I'm not sure they are mandatory for all platforms. > > > To be on the safe side, you simple need to add _optional() to the both > > > call along with above change. > > > > > > > As I know, this is the only one in spi which does not have a check for > > devm_clk_get. > > For some it still may be optional. That's why better to check it and > mention in the commit message. > > > Even if add _optional(), they still may return errors and need security > > checks. > > Of course, see "along with" in my previous comment. > Got it. I will send version 2 in which both _optional() and security checks will be added. > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH] spi: pxa2xx: Add missed security checks
On Fri, Oct 18, 2019 at 5:35 PM Andy Shevchenko wrote: > > On Fri, Oct 18, 2019 at 8:59 AM Chuhong Yuan wrote: > > > > pxa2xx_spi_init_pdata misses checks for devm_clk_get and > > platform_get_irq. > > Add checks for them to fix the bugs. > > > > Signed-off-by: Chuhong Yuan > > --- > > drivers/spi/spi-pxa2xx.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > > index bb6a14d1ab0f..2e73d75a6ac5 100644 > > --- a/drivers/spi/spi-pxa2xx.c > > +++ b/drivers/spi/spi-pxa2xx.c > > @@ -1565,7 +1565,13 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev) > > #endif > > > > ssp->clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(ssp->clk)) > > + return NULL; > > + > > ssp->irq = platform_get_irq(pdev, 0); > > + if (ssp->irq < 0) > > + return NULL; > > I'm not sure they are mandatory for all platforms. > To be on the safe side, you simple need to add _optional() to the both > call along with above change. > As I know, this is the only one in spi which does not have a check for devm_clk_get. Even if add _optional(), they still may return errors and need security checks. > -- > With Best Regards, > Andy Shevchenko
[PATCH] IB/uverbs: Add a check for uverbs_attr_get
Only uverbs_copy_to_struct_or_zero in uverbs_ioctl.c does not have a check for uverbs_attr_get. Although its usage in uverbs_response has a check for attr's validity, UVERBS_HANDLER does not. Therefore, it is better to add a check like other functions in uverbs_ioctl.c. Signed-off-by: Chuhong Yuan --- drivers/infiniband/core/uverbs_ioctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c index 61758201d9b2..269938f59d3f 100644 --- a/drivers/infiniband/core/uverbs_ioctl.c +++ b/drivers/infiniband/core/uverbs_ioctl.c @@ -795,6 +795,9 @@ int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle, { const struct uverbs_attr *attr = uverbs_attr_get(bundle, idx); + if (IS_ERR(attr)) + return PTR_ERR(attr); + if (size < attr->ptr_attr.len) { if (clear_user(u64_to_user_ptr(attr->ptr_attr.data) + size, attr->ptr_attr.len - size)) -- 2.20.1
[PATCH] ASoC: tlv320aic32x4: add a check for devm_clk_get
aic32x4_set_dai_sysclk misses a check for devm_clk_get and may miss the failure. Add a check to fix it. Signed-off-by: Chuhong Yuan --- sound/soc/codecs/tlv320aic32x4.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 68165de1c8de..b4e9a6c73f90 100644 --- a/sound/soc/codecs/tlv320aic32x4.c +++ b/sound/soc/codecs/tlv320aic32x4.c @@ -573,6 +573,9 @@ static int aic32x4_set_dai_sysclk(struct snd_soc_dai *codec_dai, struct clk *pll; pll = devm_clk_get(component->dev, "pll"); + if (IS_ERR(pll)) + return PTR_ERR(pll); + mclk = clk_get_parent(pll); return clk_set_rate(mclk, freq); -- 2.20.1
[PATCH v2] media: st-mipid02: add a check for devm_gpiod_get_optional
mipid02_probe misses a check for devm_gpiod_get_optional and may miss the failure. Add a check to fix the problem. Signed-off-by: Chuhong Yuan --- Changes in v2: - Add an error message. drivers/media/i2c/st-mipid02.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c index 81285b8d5cfb..003ba22334cd 100644 --- a/drivers/media/i2c/st-mipid02.c +++ b/drivers/media/i2c/st-mipid02.c @@ -971,6 +971,11 @@ static int mipid02_probe(struct i2c_client *client) bridge->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(bridge->reset_gpio)) { + dev_err(dev, "failed to get reset GPIO\n"); + return PTR_ERR(bridge->reset_gpio); + } + ret = mipid02_get_regulators(bridge); if (ret) { dev_err(dev, "failed to get regulators %d", ret); -- 2.20.1
Re: [PATCH] media: st-mipid02: add a check for devm_gpiod_get_optional
On Thu, Oct 17, 2019 at 1:43 PM Mickael GUENE wrote: > > Hello Chuhong, > > Is this check necessary ? > since looking into code it seems to me devm_gpiod_get_optional() can only > return NULL in case of error due to following check in > devm_gpiod_get_index_optional() > if (IS_ERR(desc)) { > if (PTR_ERR(desc) == -ENOENT) > return NULL; > } > And in that case reset_gpio is not used > The problem may not be a null return value, but a returned error, which is a minus value, like -EPROBE_DEFER or -EINVAL returned by gpiod_find in gpiod_get_index. In these cases, devm_gpiod_get_index_optional will not return null but return the error. Therefore, this check is necessary. > Regards > Mickael > > On 10/16/19 12:56, Chuhong Yuan wrote: > > mipid02_probe misses a check for devm_gpiod_get_optional and may miss > > the failure. > > Add a check to fix the problem. > > > > Signed-off-by: Chuhong Yuan > > --- > > drivers/media/i2c/st-mipid02.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c > > index 81285b8d5cfb..d38e888b0a43 100644 > > --- a/drivers/media/i2c/st-mipid02.c > > +++ b/drivers/media/i2c/st-mipid02.c > > @@ -971,6 +971,9 @@ static int mipid02_probe(struct i2c_client *client) > > bridge->reset_gpio = devm_gpiod_get_optional(dev, "reset", > >GPIOD_OUT_HIGH); > > > > + if (IS_ERR(bridge->reset_gpio)) > > + return PTR_ERR(bridge->reset_gpio); > > + > > ret = mipid02_get_regulators(bridge); > > if (ret) { > > dev_err(dev, "failed to get regulators %d", ret); > >
[PATCH] ASoC: Intel: sof-rt5682: add a check for devm_clk_get
sof_audio_probe misses a check for devm_clk_get and may cause problems. Add a check for it to fix the bug. Signed-off-by: Chuhong Yuan --- sound/soc/intel/boards/sof_rt5682.c | 9 + 1 file changed, 9 insertions(+) diff --git a/sound/soc/intel/boards/sof_rt5682.c b/sound/soc/intel/boards/sof_rt5682.c index a437567b8cee..6d15c7ff66bf 100644 --- a/sound/soc/intel/boards/sof_rt5682.c +++ b/sound/soc/intel/boards/sof_rt5682.c @@ -576,6 +576,15 @@ static int sof_audio_probe(struct platform_device *pdev) /* need to get main clock from pmc */ if (sof_rt5682_quirk & SOF_RT5682_MCLK_BYTCHT_EN) { ctx->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); + if (IS_ERR(ctx->mclk)) { + ret = PTR_ERR(ctx->mclk); + + dev_err(&pdev->dev, + "Failed to get MCLK from pmc_plt_clk_3: %d\n", + ret); + return ret; + } + ret = clk_prepare_enable(ctx->mclk); if (ret < 0) { dev_err(&pdev->dev, -- 2.20.1
[PATCH] spi: pxa2xx: Add missed security checks
pxa2xx_spi_init_pdata misses checks for devm_clk_get and platform_get_irq. Add checks for them to fix the bugs. Signed-off-by: Chuhong Yuan --- drivers/spi/spi-pxa2xx.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index bb6a14d1ab0f..2e73d75a6ac5 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1565,7 +1565,13 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev) #endif ssp->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(ssp->clk)) + return NULL; + ssp->irq = platform_get_irq(pdev, 0); + if (ssp->irq < 0) + return NULL; + ssp->type = type; ssp->pdev = pdev; ssp->port_id = pxa2xx_spi_get_port_id(adev); -- 2.20.1
Re: [PATCH 2/2] hfsplus: add a check for hfs_bnode_find
On Thu, Oct 17, 2019 at 8:07 AM Ernesto A. Fernández wrote: > > Hi, > > On Wed, Oct 16, 2019 at 08:06:20PM +0800, Chuhong Yuan wrote: > > hfs_brec_update_parent misses a check for hfs_bnode_find and may miss > > the failure. > > Add a check for it like what is done in again. > > > > Signed-off-by: Chuhong Yuan > > --- > > fs/hfsplus/brec.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c > > index 1918544a7871..22bada8288c4 100644 > > --- a/fs/hfsplus/brec.c > > +++ b/fs/hfsplus/brec.c > > @@ -434,6 +434,8 @@ static int hfs_brec_update_parent(struct hfs_find_data > > *fd) > > new_node->parent = tree->root; > > } > > fd->bnode = hfs_bnode_find(tree, new_node->parent); > > + if (IS_ERR(fd->bnode)) > > + return PTR_ERR(fd->bnode); > > You shouldn't just return here, you still hold a reference to new_node. > The call to hfs_bnode_find() after the again label seems to be making a > similar mistake. > > I don't think either one can actually fail though, because the parent > nodes have all been read and hashed before, haven't they? > I find that after hfs_bnode_findhash in hfs_bnode_find, there is a test for HFS_BNODE_ERROR and may return an error. I'm not sure whether it can happen here. > > /* create index key and entry */ > > hfs_bnode_read_key(new_node, fd->search_key, 14); > > cnid = cpu_to_be32(new_node->this); > > -- > > 2.20.1 > >
[PATCH] staging: iio: ad9834: add a check for devm_clk_get
ad9834_probe misses a check for devm_clk_get and may cause problems. Add a check like what ad9832 does to fix it. Signed-off-by: Chuhong Yuan --- drivers/staging/iio/frequency/ad9834.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c index 038d6732c3fd..23026978a5a5 100644 --- a/drivers/staging/iio/frequency/ad9834.c +++ b/drivers/staging/iio/frequency/ad9834.c @@ -417,6 +417,10 @@ static int ad9834_probe(struct spi_device *spi) st = iio_priv(indio_dev); mutex_init(&st->lock); st->mclk = devm_clk_get(&spi->dev, NULL); + if (IS_ERR(st->mclk)) { + ret = PTR_ERR(st->mclk); + goto error_disable_reg; + } ret = clk_prepare_enable(st->mclk); if (ret) { -- 2.20.1
[PATCH v2] leds: an30259a: add a check for devm_regmap_init_i2c
an30259a_probe misses a check for devm_regmap_init_i2c and may cause problems. Add a check and print errors like other leds drivers. Signed-off-by: Chuhong Yuan --- Changes in v2: - Use goto exit instead of return to destroy the mutex when failed. drivers/leds/leds-an30259a.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c index 250dc9d6f635..82350a28a564 100644 --- a/drivers/leds/leds-an30259a.c +++ b/drivers/leds/leds-an30259a.c @@ -305,6 +305,13 @@ static int an30259a_probe(struct i2c_client *client) chip->regmap = devm_regmap_init_i2c(client, &an30259a_regmap_config); + if (IS_ERR(chip->regmap)) { + err = PTR_ERR(chip->regmap); + dev_err(&client->dev, "Failed to allocate register map: %d\n", + err); + goto exit; + } + for (i = 0; i < chip->num_leds; i++) { struct led_init_data init_data = {}; -- 2.20.1
[PATCH] clocksource: asm9260: Add a check for of_clk_get
asm9260_timer_init misses a check for of_clk_get. Add a check for it and print errors like other clocksource drivers. Signed-off-by: Chuhong Yuan --- drivers/clocksource/asm9260_timer.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/clocksource/asm9260_timer.c b/drivers/clocksource/asm9260_timer.c index 9f09a59161e7..5b39d3701fa3 100644 --- a/drivers/clocksource/asm9260_timer.c +++ b/drivers/clocksource/asm9260_timer.c @@ -194,6 +194,10 @@ static int __init asm9260_timer_init(struct device_node *np) } clk = of_clk_get(np, 0); + if (IS_ERR(clk)) { + pr_err("Failed to get clk!\n"); + return PTR_ERR(clk); + } ret = clk_prepare_enable(clk); if (ret) { -- 2.20.1
[PATCH 2/2] hfsplus: add a check for hfs_bnode_find
hfs_brec_update_parent misses a check for hfs_bnode_find and may miss the failure. Add a check for it like what is done in again. Signed-off-by: Chuhong Yuan --- fs/hfsplus/brec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c index 1918544a7871..22bada8288c4 100644 --- a/fs/hfsplus/brec.c +++ b/fs/hfsplus/brec.c @@ -434,6 +434,8 @@ static int hfs_brec_update_parent(struct hfs_find_data *fd) new_node->parent = tree->root; } fd->bnode = hfs_bnode_find(tree, new_node->parent); + if (IS_ERR(fd->bnode)) + return PTR_ERR(fd->bnode); /* create index key and entry */ hfs_bnode_read_key(new_node, fd->search_key, 14); cnid = cpu_to_be32(new_node->this); -- 2.20.1
[PATCH] leds: an30259a: add a check for devm_regmap_init_i2c
an30259a_probe misses a check for devm_regmap_init_i2c and may cause problems. Add a check and print errors like other leds drivers. Signed-off-by: Chuhong Yuan --- drivers/leds/leds-an30259a.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c index 250dc9d6f635..03ad068f29f7 100644 --- a/drivers/leds/leds-an30259a.c +++ b/drivers/leds/leds-an30259a.c @@ -305,6 +305,13 @@ static int an30259a_probe(struct i2c_client *client) chip->regmap = devm_regmap_init_i2c(client, &an30259a_regmap_config); + if (IS_ERR(chip->regmap)) { + err = PTR_ERR(chip->regmap); + dev_err(&client->dev, "Failed to allocate register map: %d\n", + err); + return err; + } + for (i = 0; i < chip->num_leds; i++) { struct led_init_data init_data = {}; -- 2.20.1
[PATCH 1/2] hfs: add a check for hfs_bnode_find
hfs_brec_update_parent misses a check for hfs_bnode_find and may miss the failure. Add a check for it like what is done in again. Signed-off-by: Chuhong Yuan --- fs/hfs/brec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/hfs/brec.c b/fs/hfs/brec.c index 896396554bcc..e6af05a4cd1b 100644 --- a/fs/hfs/brec.c +++ b/fs/hfs/brec.c @@ -430,6 +430,8 @@ static int hfs_brec_update_parent(struct hfs_find_data *fd) new_node->parent = tree->root; } fd->bnode = hfs_bnode_find(tree, new_node->parent); + if (IS_ERR(fd->bnode)) + return PTR_ERR(fd->bnode); /* create index key and entry */ hfs_bnode_read_key(new_node, fd->search_key, 14); cnid = cpu_to_be32(new_node->this); -- 2.20.1
[PATCH] media: st-mipid02: add a check for devm_gpiod_get_optional
mipid02_probe misses a check for devm_gpiod_get_optional and may miss the failure. Add a check to fix the problem. Signed-off-by: Chuhong Yuan --- drivers/media/i2c/st-mipid02.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c index 81285b8d5cfb..d38e888b0a43 100644 --- a/drivers/media/i2c/st-mipid02.c +++ b/drivers/media/i2c/st-mipid02.c @@ -971,6 +971,9 @@ static int mipid02_probe(struct i2c_client *client) bridge->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(bridge->reset_gpio)) + return PTR_ERR(bridge->reset_gpio); + ret = mipid02_get_regulators(bridge); if (ret) { dev_err(dev, "failed to get regulators %d", ret); -- 2.20.1
[PATCH] USB: bcma: Add a check for devm_gpiod_get
bcma_hcd_probe misses a check for devm_gpiod_get and may miss the error. Add a check for it and return the error if a failure occurs. Signed-off-by: Chuhong Yuan --- drivers/usb/host/bcma-hcd.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/bcma-hcd.c b/drivers/usb/host/bcma-hcd.c index 2400a826397a..652fa29beb27 100644 --- a/drivers/usb/host/bcma-hcd.c +++ b/drivers/usb/host/bcma-hcd.c @@ -406,9 +406,12 @@ static int bcma_hcd_probe(struct bcma_device *core) return -ENOMEM; usb_dev->core = core; - if (core->dev.of_node) + if (core->dev.of_node) { usb_dev->gpio_desc = devm_gpiod_get(&core->dev, "vcc", GPIOD_OUT_HIGH); + if (IS_ERR(usb_dev->gpio_desc)) + return PTR_ERR(usb_dev->gpio_desc); + } switch (core->id.id) { case BCMA_CORE_USB20_HOST: -- 2.20.1
[PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get
devm_regulator_get may return an error but mipi_csis_phy_init misses a check for it. This may lead to problems when regulator_set_voltage uses the unchecked pointer. This patch adds a check for devm_regulator_get to avoid potential risk. Signed-off-by: Chuhong Yuan --- Changes in v2: - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init. drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c index 73d8354e618c..e8a6acaa969e 100644 --- a/drivers/staging/media/imx/imx7-mipi-csis.c +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) static int mipi_csis_phy_init(struct csi_state *state) { state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); + if (IS_ERR(state->mipi_phy_regulator)) + return PTR_ERR(state->mipi_phy_regulator); return regulator_set_voltage(state->mipi_phy_regulator, 100, 100); @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev) return ret; } - mipi_csis_phy_init(state); + ret = mipi_csis_phy_init(state); + if (ret < 0) + return ret; + mipi_csis_phy_reset(state); mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); -- 2.20.1
[PATCH] cifs: Fix missed free operations
cifs_setattr_nounix has two paths which miss free operations for xid and fullpath. Use goto cifs_setattr_exit like other paths to fix them. Fixes: aa081859b10c ("cifs: flush before set-info if we have writeable handles") Signed-off-by: Chuhong Yuan --- fs/cifs/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 5dcc95b38310..df9377828e2f 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -2475,9 +2475,9 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) rc = tcon->ses->server->ops->flush(xid, tcon, &wfile->fid); cifsFileInfo_put(wfile); if (rc) - return rc; + goto cifs_setattr_exit; } else if (rc != -EBADF) - return rc; + goto cifs_setattr_exit; else rc = 0; } -- 2.20.1
[PATCH] media: imx7-mipi-csis: Add a check for devm_regulator_get
devm_regulator_get may return an error but mipi_csis_phy_init misses a check for it. This may lead to problems when regulator_set_voltage uses the unchecked pointer. This patch adds a check for devm_regulator_get to avoid potential risk. Signed-off-by: Chuhong Yuan --- drivers/staging/media/imx/imx7-mipi-csis.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c index 73d8354e618c..9a07b54c4ab1 100644 --- a/drivers/staging/media/imx/imx7-mipi-csis.c +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state) static int mipi_csis_phy_init(struct csi_state *state) { state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); + if (IS_ERR(state->mipi_phy_regulator)) + return PTR_ERR(state->mipi_phy_regulator); return regulator_set_voltage(state->mipi_phy_regulator, 100, 100); -- 2.20.1
Re: [PATCH 01/12] rdmacg: Replace strncmp with str_has_prefix
On Fri, Aug 30, 2019 at 7:52 AM Kees Cook wrote: > > On Tue, Jul 30, 2019 at 02:39:40PM +0800, Chuhong Yuan wrote: > > I think with the help of Coccinelle script, all strncmp(str, const, len) > > can be replaced and these problems will be eliminated. :) > > Hi! Just pinging this thread again. Any progress on this conversion? > I didn't work further on this conversion since it seems that developers are not very interested in this problem (only half of my sent patches have been responded till now) and I am working on other projects recently. If the Coccinelle script is needed, I can try to implement it next week. Regards, Chuhong > Thanks! > > -- > Kees Cook
[PATCH v2] usb: chipidea: msm: Use device-managed registration API
Use devm_reset_controller_register to get rid of manual unregistration. Signed-off-by: Chuhong Yuan --- Changes in v2: - Remove not needed err_fs. drivers/usb/chipidea/ci_hdrc_msm.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index bb4645a8ca46..af648ba6544d 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -216,13 +216,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) ci->rcdev.ops = &ci_hdrc_msm_reset_ops; ci->rcdev.of_node = pdev->dev.of_node; ci->rcdev.nr_resets = 2; - ret = reset_controller_register(&ci->rcdev); + ret = devm_reset_controller_register(&pdev->dev, &ci->rcdev); if (ret) return ret; ret = clk_prepare_enable(ci->fs_clk); if (ret) - goto err_fs; + return ret; reset_control_assert(reset); usleep_range(1, 12000); @@ -232,7 +232,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) ret = clk_prepare_enable(ci->core_clk); if (ret) - goto err_fs; + return ret; ret = clk_prepare_enable(ci->iface_clk); if (ret) @@ -271,8 +271,6 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) clk_disable_unprepare(ci->iface_clk); err_iface: clk_disable_unprepare(ci->core_clk); -err_fs: - reset_controller_unregister(&ci->rcdev); return ret; } @@ -284,7 +282,6 @@ static int ci_hdrc_msm_remove(struct platform_device *pdev) ci_hdrc_remove_device(ci->ci); clk_disable_unprepare(ci->iface_clk); clk_disable_unprepare(ci->core_clk); - reset_controller_unregister(&ci->rcdev); return 0; } -- 2.20.1
Re: [PATCH] usb: chipidea: msm: Use device-managed registration API
On Wed, Aug 28, 2019 at 11:24 AM Peter Chen wrote: > > On 19-07-23 11:02:07, Chuhong Yuan wrote: > > Use devm_reset_controller_register to get rid > > of manual unregistration. > > > > Signed-off-by: Chuhong Yuan > > --- > > drivers/usb/chipidea/ci_hdrc_msm.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c > > b/drivers/usb/chipidea/ci_hdrc_msm.c > > index bb4645a8ca46..067542e84aea 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_msm.c > > +++ b/drivers/usb/chipidea/ci_hdrc_msm.c > > @@ -216,7 +216,7 @@ static int ci_hdrc_msm_probe(struct platform_device > > *pdev) > > ci->rcdev.ops = &ci_hdrc_msm_reset_ops; > > ci->rcdev.of_node = pdev->dev.of_node; > > ci->rcdev.nr_resets = 2; > > - ret = reset_controller_register(&ci->rcdev); > > + ret = devm_reset_controller_register(&pdev->dev, &ci->rcdev); > > if (ret) > > return ret; > > > > @@ -272,7 +272,6 @@ static int ci_hdrc_msm_probe(struct platform_device > > *pdev) > > err_iface: > > clk_disable_unprepare(ci->core_clk); > > err_fs: > > - reset_controller_unregister(&ci->rcdev); > > It is devm API, why the unregister needs to be called at > fail path? > I am not very clear about your problem... After using devm_reset_controller_register(), I have removed reset_controller_unregister() calls in this patch. > Peter > > > return ret; > > } > > > > @@ -284,7 +283,6 @@ static int ci_hdrc_msm_remove(struct platform_device > > *pdev) > > ci_hdrc_remove_device(ci->ci); > > clk_disable_unprepare(ci->iface_clk); > > clk_disable_unprepare(ci->core_clk); > > - reset_controller_unregister(&ci->rcdev); > > > > return 0; > > } > > -- > > 2.20.1 > >
Re: [PATCH v4 4/8] printk: Replace strncmp with str_has_prefix
On Thu, Aug 15, 2019 at 9:52 PM Petr Mladek wrote: > > On Thu 2019-08-15 16:50:33, Sergey Senozhatsky wrote: > > On (08/14/19 12:49), Petr Mladek wrote: > > > On Fri 2019-08-09 15:10:34, Chuhong Yuan wrote: > > > > strncmp(str, const, len) is error-prone because len > > > > is easy to have typo. > > > > The example is the hard-coded len has counting error > > > > or sizeof(const) forgets - 1. > > > > So we prefer using newly introduced str_has_prefix() > > > > to substitute such strncmp to make code better. > > > > > > > > Signed-off-by: Chuhong Yuan > > > > > > Reviewed-by: Petr Mladek > > > > Reviewed-by: Sergey Senozhatsky > > Chuong Yuan, should I take this patch via printk.git? Or will > the entire series go via some other tree, please? > I think that it goes via printk.git is okay, thanks! > Best Regards, > Petr
Re: [PATCH 3/3] xen/blkback: Use refcount_t for refcount
On Thu, Aug 8, 2019 at 9:35 PM Roger Pau Monné wrote: > > On Thu, Aug 08, 2019 at 09:11:00PM +0800, Chuhong Yuan wrote: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > Thanks! > > I think there are more reference counters in blkback than > the one you fixed. There's also an inflight field in xen_blkif_ring, > and a pendcnt in pending_req which look like possible candidates to > switch to use refcount_t, have you looked into switching those two > also? > It seems that xen_blkif_ring::inflight is 0-based and cannot be directly converted to refcount_t. This is because the implementation of refcount_t will warn on increasing a 0 ref count. Therefore I only convert pending_req::pendcnt in v2. > Roger.
Re: [PATCH] regulator: core: Add devres versions of regulator_enable/disable
On Mon, Aug 12, 2019 at 8:51 PM Chuhong Yuan wrote: > > On Mon, Aug 12, 2019 at 7:07 PM Mark Brown wrote: > > > > On Sat, Aug 10, 2019 at 09:44:45AM +0800, Chuhong Yuan wrote: > > > On Fri, Aug 9, 2019 at 11:11 PM Mark Brown wrote: > > > > > > I'm not super keen on managed versions of these functions since they're > > > > very likely to cause reference counting issues between the probe/remove > > > > path and the suspend/resume path which aren't obvious from the code, I'm > > > > especially worried about double frees on release. > > > > > I find that 29 of 31 cases I found call regulator_disable() only when > > > encounter > > > probe failure or in .remove. > > > So I think the devm versions of regulator_enable/disable() will not cause > > > big > > > problems. > > > > There's way more drivers using regulators than that... > > > > I wrote a new coccinelle script to detect all regulator_disable() in .remove, > 101 drivers are found in total. > Within them, 25 drivers cannot benefit from devres version of > regulator_enable() > since they have additional non-devm operations after > regulator_disable() in .remove. I find 6 of 25 can benefit from devm_regulator_enable(). They are included in my previously found 147 cases so I incorrectly skipped them while checking. Therefore, there are 82 cases that can benefit from devm_regulator_enable() and 66 of them(80.5%) only call regulator_disable() when fail in probe or in .remove. > Within the left 76 cases, 60 drivers (79%) only use > regulator_disable() when encounter > probe failure or in .remove. > The left 16 cases mostly use regulator_disable() in _suspend(). > Furthermore, 3 cases of 76 are found to forget to disable regulator > when fail in probe. > So I think a devres version of regulator_enable/disable() has more > benefits than potential > risk. > > > > I even found a driver to forget to disable regulator when encounter > > > probe failure, > > > which is drivers/iio/adc/ti-adc128s052.c. > > > And a devm version of regulator_enable() can prevent such mistakes. > > > > Yes, it's useful for that.
Re: [PATCH] regulator: core: Add devres versions of regulator_enable/disable
On Mon, Aug 12, 2019 at 7:07 PM Mark Brown wrote: > > On Sat, Aug 10, 2019 at 09:44:45AM +0800, Chuhong Yuan wrote: > > On Fri, Aug 9, 2019 at 11:11 PM Mark Brown wrote: > > > > I'm not super keen on managed versions of these functions since they're > > > very likely to cause reference counting issues between the probe/remove > > > path and the suspend/resume path which aren't obvious from the code, I'm > > > especially worried about double frees on release. > > > I find that 29 of 31 cases I found call regulator_disable() only when > > encounter > > probe failure or in .remove. > > So I think the devm versions of regulator_enable/disable() will not cause > > big > > problems. > > There's way more drivers using regulators than that... > I wrote a new coccinelle script to detect all regulator_disable() in .remove, 101 drivers are found in total. Within them, 25 drivers cannot benefit from devres version of regulator_enable() since they have additional non-devm operations after regulator_disable() in .remove. Within the left 76 cases, 60 drivers (79%) only use regulator_disable() when encounter probe failure or in .remove. The left 16 cases mostly use regulator_disable() in _suspend(). Furthermore, 3 cases of 76 are found to forget to disable regulator when fail in probe. So I think a devres version of regulator_enable/disable() has more benefits than potential risk. > > I even found a driver to forget to disable regulator when encounter > > probe failure, > > which is drivers/iio/adc/ti-adc128s052.c. > > And a devm version of regulator_enable() can prevent such mistakes. > > Yes, it's useful for that.
[PATCH v2] iio: adc: max1027: Use device-managed APIs
Use device-managed APIs to simplify the code. Signed-off-by: Chuhong Yuan --- Changes in v2: - Delete the debug print and _remove. drivers/iio/adc/max1027.c | 38 +++--- 1 file changed, 7 insertions(+), 31 deletions(-) diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c index da84adfdb819..214883458582 100644 --- a/drivers/iio/adc/max1027.c +++ b/drivers/iio/adc/max1027.c @@ -427,8 +427,9 @@ static int max1027_probe(struct spi_device *spi) return -ENOMEM; } - ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, -&max1027_trigger_handler, NULL); + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, + &iio_pollfunc_store_time, + &max1027_trigger_handler, NULL); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to setup buffer\n"); return ret; @@ -439,7 +440,7 @@ static int max1027_probe(struct spi_device *spi) if (st->trig == NULL) { ret = -ENOMEM; dev_err(&indio_dev->dev, "Failed to allocate iio trigger\n"); - goto fail_trigger_alloc; + return ret; } st->trig->ops = &max1027_trigger_ops; @@ -454,7 +455,7 @@ static int max1027_probe(struct spi_device *spi) spi->dev.driver->name, st->trig); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to allocate IRQ.\n"); - goto fail_dev_register; + return ret; } /* Disable averaging */ @@ -462,34 +463,10 @@ static int max1027_probe(struct spi_device *spi) ret = spi_write(st->spi, &st->reg, 1); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to configure averaging register\n"); - goto fail_dev_register; - } - - ret = iio_device_register(indio_dev); - if (ret < 0) { - dev_err(&indio_dev->dev, "Failed to register iio device\n"); - goto fail_dev_register; + return ret; } - return 0; - -fail_dev_register: -fail_trigger_alloc: - iio_triggered_buffer_cleanup(indio_dev); - - return ret; -} - -static int max1027_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - - pr_debug("%s: remove(spi = 0x%p)\n", __func__, spi); - - iio_device_unregister(indio_dev); - iio_triggered_buffer_cleanup(indio_dev); - - return 0; + return devm_iio_device_register(&spi->dev, indio_dev); } static struct spi_driver max1027_driver = { @@ -498,7 +475,6 @@ static struct spi_driver max1027_driver = { .of_match_table = of_match_ptr(max1027_adc_dt_ids), }, .probe = max1027_probe, - .remove = max1027_remove, .id_table = max1027_id, }; module_spi_driver(max1027_driver); -- 2.20.1
Re: [PATCH] regulator: core: Add devres versions of regulator_enable/disable
On Fri, Aug 9, 2019 at 11:11 PM Mark Brown wrote: > > On Fri, Aug 09, 2019 at 11:03:52AM +0800, Chuhong Yuan wrote: > > I wrote a coccinelle script to detect possible chances > > of utilizing devm_() APIs to simplify the driver. > > The script found 147 drivers in total and 22 of them > > have be patched. > > > Within the 125 left ones, at least 31 of them (24.8%) > > are hindered from benefiting from devm_() APIs because > > of lack of a devres version of regulator_enable(). > > I'm not super keen on managed versions of these functions since they're > very likely to cause reference counting issues between the probe/remove > path and the suspend/resume path which aren't obvious from the code, I'm > especially worried about double frees on release. I find that 29 of 31 cases I found call regulator_disable() only when encounter probe failure or in .remove. So I think the devm versions of regulator_enable/disable() will not cause big problems. I even found a driver to forget to disable regulator when encounter probe failure, which is drivers/iio/adc/ti-adc128s052.c. And a devm version of regulator_enable() can prevent such mistakes.
Re: [PATCH] powerpc/mm: Use refcount_t for refcount
On Fri, Aug 9, 2019 at 8:36 PM Michael Ellerman wrote: > > Chuhong Yuan writes: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > > > Signed-off-by: Chuhong Yuan > > Thanks. > > We don't have a fast implementation of refcount_t, so I'm worried this > could cause a measurable performance regression. > > Did you benchmark it at all? > I did not benchmark it and I don't have the testing environment... > cheers > > > diff --git a/arch/powerpc/mm/book3s64/mmu_context.c > > b/arch/powerpc/mm/book3s64/mmu_context.c > > index 2d0cb5ba9a47..f836fd5a6abc 100644 > > --- a/arch/powerpc/mm/book3s64/mmu_context.c > > +++ b/arch/powerpc/mm/book3s64/mmu_context.c > > @@ -231,7 +231,7 @@ static void pmd_frag_destroy(void *pmd_frag) > > /* drop all the pending references */ > > count = ((unsigned long)pmd_frag & ~PAGE_MASK) >> PMD_FRAG_SIZE_SHIFT; > > /* We allow PTE_FRAG_NR fragments from a PTE page */ > > - if (atomic_sub_and_test(PMD_FRAG_NR - count, > > &page->pt_frag_refcount)) { > > + if (refcount_sub_and_test(PMD_FRAG_NR - count, > > &page->pt_frag_refcount)) { > > pgtable_pmd_page_dtor(page); > > __free_page(page); > > } > > diff --git a/arch/powerpc/mm/book3s64/pgtable.c > > b/arch/powerpc/mm/book3s64/pgtable.c > > index 7d0e0d0d22c4..40056896ce4e 100644 > > --- a/arch/powerpc/mm/book3s64/pgtable.c > > +++ b/arch/powerpc/mm/book3s64/pgtable.c > > @@ -277,7 +277,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm) > > return NULL; > > } > > > > - atomic_set(&page->pt_frag_refcount, 1); > > + refcount_set(&page->pt_frag_refcount, 1); > > > > ret = page_address(page); > > /* > > @@ -294,7 +294,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm) > >* count. > >*/ > > if (likely(!mm->context.pmd_frag)) { > > - atomic_set(&page->pt_frag_refcount, PMD_FRAG_NR); > > + refcount_set(&page->pt_frag_refcount, PMD_FRAG_NR); > > mm->context.pmd_frag = ret + PMD_FRAG_SIZE; > > } > > spin_unlock(&mm->page_table_lock); > > @@ -317,8 +317,7 @@ void pmd_fragment_free(unsigned long *pmd) > > { > > struct page *page = virt_to_page(pmd); > > > > - BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0); > > - if (atomic_dec_and_test(&page->pt_frag_refcount)) { > > + if (refcount_dec_and_test(&page->pt_frag_refcount)) { > > pgtable_pmd_page_dtor(page); > > __free_page(page); > > } > > diff --git a/arch/powerpc/mm/pgtable-frag.c b/arch/powerpc/mm/pgtable-frag.c > > index a7b05214760c..4ef8231b677f 100644 > > --- a/arch/powerpc/mm/pgtable-frag.c > > +++ b/arch/powerpc/mm/pgtable-frag.c > > @@ -24,7 +24,7 @@ void pte_frag_destroy(void *pte_frag) > > /* drop all the pending references */ > > count = ((unsigned long)pte_frag & ~PAGE_MASK) >> PTE_FRAG_SIZE_SHIFT; > > /* We allow PTE_FRAG_NR fragments from a PTE page */ > > - if (atomic_sub_and_test(PTE_FRAG_NR - count, > > &page->pt_frag_refcount)) { > > + if (refcount_sub_and_test(PTE_FRAG_NR - count, > > &page->pt_frag_refcount)) { > > pgtable_page_dtor(page); > > __free_page(page); > > } > > @@ -71,7 +71,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, > > int kernel) > > return NULL; > > } > > > > - atomic_set(&page->pt_frag_refcount, 1); > > + refcount_set(&page->pt_frag_refcount, 1); > > > > ret = page_address(page); > > /* > > @@ -87,7 +87,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, > > int kernel) > >* count. > >*/ > > if (likely(!pte_frag_get(&mm->context))) { > > - atomic_set(&page->pt_frag_refcount, PTE_FRAG_NR); > > + refcount_set(&page->pt_frag_refcount, PTE_FRAG_NR); > > pte_frag_set(&mm->context, ret + PTE_FRAG_SIZE); > > } > > spin_unlock(&mm->page_table_lock); > > @@
Re: [PATCH v4 6/8] sched: Replace strncmp with str_has_prefix
On Fri, Aug 9, 2019 at 7:31 PM Valentin Schneider wrote: > > On 09/08/2019 08:10, Chuhong Yuan wrote: > > strncmp(str, const, len) is error-prone because len > > is easy to have typo. > > The example is the hard-coded len has counting error > > or sizeof(const) forgets - 1. > > So we prefer using newly introduced str_has_prefix() > > to substitute such strncmp to make code better. > > > > Signed-off-by: Chuhong Yuan > > I tried to have a look at the series as a whole but it's not properly > threaded (or at least doesn't appear as such on lore), which makes it > unnecessarily annoying to review. > > Please make sure to use git-send-email, which should properly thread all > patches (IOW make them in-reply-to the cover letter). > I have used git-send-email to send the series with a cover letter. The cover letter is here: https://lkml.org/lkml/2019/8/9/108 Indeed I find the series are not in the same thread, I am not sure what is wrong with them. > > Other than that, I stared at it and it seems fine. It's not that helpful > here since I doubt any of these prefixes will change in the near feature, > but hey, why not. > > Reviewed-by: Valentin Schneider > > > --- > > Changes in v4: > > - Eliminate assignments in if conditions. > > > > kernel/sched/debug.c | 6 -- > > kernel/sched/isolation.c | 11 +++ > > 2 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c > > index f7e4579e746c..a03900523e5d 100644 > > --- a/kernel/sched/debug.c > > +++ b/kernel/sched/debug.c > > @@ -102,10 +102,12 @@ static int sched_feat_set(char *cmp) > > { > > int i; > > int neg = 0; > > + size_t len; > > > > - if (strncmp(cmp, "NO_", 3) == 0) { > > + len = str_has_prefix(cmp, "NO_"); > > + if (len) { > > neg = 1; > > - cmp += 3; > > + cmp += len; > > } > > > > i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp); > > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c > > index ccb28085b114..ea2ead4b1906 100644 > > --- a/kernel/sched/isolation.c > > +++ b/kernel/sched/isolation.c > > @@ -141,16 +141,19 @@ __setup("nohz_full=", housekeeping_nohz_full_setup); > > static int __init housekeeping_isolcpus_setup(char *str) > > { > > unsigned int flags = 0; > > + size_t len; > > > > while (isalpha(*str)) { > > - if (!strncmp(str, "nohz,", 5)) { > > - str += 5; > > + len = str_has_prefix(str, "nohz,"); > > + if (len) { > > + str += len; > > flags |= HK_FLAG_TICK; > > continue; > > } > > > > - if (!strncmp(str, "domain,", 7)) { > > - str += 7; > > + len = str_has_prefix(str, "domain,"); > > + if (len) { > > + str += len; > > flags |= HK_FLAG_DOMAIN; > > continue; > > } > >
[PATCH v4 8/8] watchdog: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- kernel/watchdog.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 7f9e7b9306fe..ac7a4b5f856e 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -70,13 +70,13 @@ void __init hardlockup_detector_disable(void) static int __init hardlockup_panic_setup(char *str) { - if (!strncmp(str, "panic", 5)) + if (str_has_prefix(str, "panic")) hardlockup_panic = 1; - else if (!strncmp(str, "nopanic", 7)) + else if (str_has_prefix(str, "nopanic")) hardlockup_panic = 0; - else if (!strncmp(str, "0", 1)) + else if (str_has_prefix(str, "0")) nmi_watchdog_user_enabled = 0; - else if (!strncmp(str, "1", 1)) + else if (str_has_prefix(str, "1")) nmi_watchdog_user_enabled = 1; return 1; } -- 2.20.1
[PATCH v4 7/8] userns: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v4: - Eliminate assignments in if conditions. kernel/user_namespace.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 8eadadc478f9..bd5702f9273a 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -1138,6 +1138,7 @@ ssize_t proc_setgroups_write(struct file *file, const char __user *buf, char kbuf[8], *pos; bool setgroups_allowed; ssize_t ret; + size_t len; /* Only allow a very narrow range of strings to be written */ ret = -EINVAL; @@ -1153,16 +1154,19 @@ ssize_t proc_setgroups_write(struct file *file, const char __user *buf, /* What is being requested? */ ret = -EINVAL; - if (strncmp(pos, "allow", 5) == 0) { - pos += 5; + + len = str_has_prefix(pos, "allow"); + if (len) { + pos += len; setgroups_allowed = true; + } else { + len = str_has_prefix(pos, "deny"); + if (len) { + pos += len; + setgroups_allowed = false; + } else + goto out; } - else if (strncmp(pos, "deny", 4) == 0) { - pos += 4; - setgroups_allowed = false; - } - else - goto out; /* Verify there is not trailing junk on the line */ pos = skip_spaces(pos); -- 2.20.1
[PATCH v4 4/8] printk: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v4: - Eliminate assignments in if conditions. kernel/printk/braille.c | 15 +++ kernel/printk/printk.c | 22 -- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c index 1d21ebacfdb8..17a9591e54ff 100644 --- a/kernel/printk/braille.c +++ b/kernel/printk/braille.c @@ -11,11 +11,18 @@ int _braille_console_setup(char **str, char **brl_options) { - if (!strncmp(*str, "brl,", 4)) { + size_t len; + + len = str_has_prefix(*str, "brl,"); + if (len) { *brl_options = ""; - *str += 4; - } else if (!strncmp(*str, "brl=", 4)) { - *brl_options = *str + 4; + *str += len; + return 0; + } + + len = str_has_prefix(*str, "brl="); + if (len) { + *brl_options = *str + len; *str = strchr(*brl_options, ','); if (!*str) { pr_err("need port name after brl=\n"); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 1888f6a3b694..43a31015ec93 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -118,19 +118,29 @@ static unsigned int __read_mostly devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT; static int __control_devkmsg(char *str) { + size_t len; + if (!str) return -EINVAL; - if (!strncmp(str, "on", 2)) { + len = str_has_prefix(str, "on"); + if (len) { devkmsg_log = DEVKMSG_LOG_MASK_ON; - return 2; - } else if (!strncmp(str, "off", 3)) { + return len; + } + + len = str_has_prefix(str, "off"); + if (len) { devkmsg_log = DEVKMSG_LOG_MASK_OFF; - return 3; - } else if (!strncmp(str, "ratelimit", 9)) { + return len; + } + + len = str_has_prefix(str, "ratelimit"); + if (len) { devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT; - return 9; + return len; } + return -EINVAL; } -- 2.20.1
[PATCH v4 2/8] module: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- kernel/module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index 5933395af9a0..7defa2a4a701 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2251,7 +2251,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info) switch (sym[i].st_shndx) { case SHN_COMMON: /* Ignore common symbols */ - if (!strncmp(name, "__gnu_lto", 9)) + if (str_has_prefix(name, "__gnu_lto")) break; /* We compiled with -fno-common. These are not -- 2.20.1
[PATCH v4 6/8] sched: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v4: - Eliminate assignments in if conditions. kernel/sched/debug.c | 6 -- kernel/sched/isolation.c | 11 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index f7e4579e746c..a03900523e5d 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -102,10 +102,12 @@ static int sched_feat_set(char *cmp) { int i; int neg = 0; + size_t len; - if (strncmp(cmp, "NO_", 3) == 0) { + len = str_has_prefix(cmp, "NO_"); + if (len) { neg = 1; - cmp += 3; + cmp += len; } i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp); diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c index ccb28085b114..ea2ead4b1906 100644 --- a/kernel/sched/isolation.c +++ b/kernel/sched/isolation.c @@ -141,16 +141,19 @@ __setup("nohz_full=", housekeeping_nohz_full_setup); static int __init housekeeping_isolcpus_setup(char *str) { unsigned int flags = 0; + size_t len; while (isalpha(*str)) { - if (!strncmp(str, "nohz,", 5)) { - str += 5; + len = str_has_prefix(str, "nohz,"); + if (len) { + str += len; flags |= HK_FLAG_TICK; continue; } - if (!strncmp(str, "domain,", 7)) { - str += 7; + len = str_has_prefix(str, "domain,"); + if (len) { + str += len; flags |= HK_FLAG_DOMAIN; continue; } -- 2.20.1
[PATCH v4 5/8] reboot: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v4: - Eliminate assignments in if conditions. kernel/reboot.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kernel/reboot.c b/kernel/reboot.c index c4d472b7f1b4..c79aaac43785 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -520,6 +520,8 @@ EXPORT_SYMBOL_GPL(orderly_reboot); static int __init reboot_setup(char *str) { + size_t len; + for (;;) { enum reboot_mode *mode; @@ -530,9 +532,10 @@ static int __init reboot_setup(char *str) */ reboot_default = 0; - if (!strncmp(str, "panic_", 6)) { + len = str_has_prefix(str, "panic_"); + if (len) { mode = &panic_reboot_mode; - str += 6; + str += len; } else { mode = &reboot_mode; } -- 2.20.1
[PATCH v4 3/8] PM/sleep: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- kernel/power/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/power/main.c b/kernel/power/main.c index bdbd605c4215..5e5f64bb3a43 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -495,7 +495,7 @@ static suspend_state_t decode_state(const char *buf, size_t n) len = p ? p - buf : n; /* Check hibernation first. */ - if (len == 4 && !strncmp(buf, "disk", len)) + if (len == 4 && str_has_prefix(buf, "disk")) return PM_SUSPEND_MAX; #ifdef CONFIG_SUSPEND -- 2.20.1
[PATCH v4 0/8] Replace strncmp with str_has_prefix
The commit 72921427d46b ("string.h: Add str_has_prefix() helper function") introduced str_has_prefix() to substitute error-prone strncmp(str, const, len). strncmp(str, const, len) is easy to have error in len because of counting error or sizeof(const) without - 1. These patches replace such pattern with str_has_prefix() to avoid hard coded constant length and sizeof. Besides, str_has_prefix() returns the length of prefix when the comparison returns true. We can use this return value to substitute some hard-coding. Changelog: v1 -> v2: - Revise the description. - Use the return value of str_has_prefix() to eliminate hard coding. - Remove possible false positives and add newly detected one in upstream. v2 -> v3: - Revise the description. - Remove else uses in printk.c. v3 -> v4: - Eliminate assignments in if conditions. Chuhong Yuan (8): dma: debug: Replace strncmp with str_has_prefix module: Replace strncmp with str_has_prefix PM/sleep: Replace strncmp with str_has_prefix printk: Replace strncmp with str_has_prefix reboot: Replace strncmp with str_has_prefix sched: Replace strncmp with str_has_prefix userns: Replace strncmp with str_has_prefix watchdog: Replace strncmp with str_has_prefix kernel/dma/debug.c | 2 +- kernel/module.c | 2 +- kernel/power/main.c | 2 +- kernel/printk/braille.c | 15 +++ kernel/printk/printk.c | 22 -- kernel/reboot.c | 7 +-- kernel/sched/debug.c | 6 -- kernel/sched/isolation.c | 11 +++ kernel/user_namespace.c | 20 kernel/watchdog.c| 8 10 files changed, 62 insertions(+), 33 deletions(-) -- 2.20.1
[PATCH] regulator: core: Add devres versions of regulator_enable/disable
I wrote a coccinelle script to detect possible chances of utilizing devm_() APIs to simplify the driver. The script found 147 drivers in total and 22 of them have be patched. Within the 125 left ones, at least 31 of them (24.8%) are hindered from benefiting from devm_() APIs because of lack of a devres version of regulator_enable(). Therefore I implemented devm_regulator_enable/disable() to make more drivers possible to use devm_() APIs. Signed-off-by: Chuhong Yuan --- drivers/regulator/devres.c | 55 ++ 1 file changed, 55 insertions(+) diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c index 3ea1c170f840..507151a71fd3 100644 --- a/drivers/regulator/devres.c +++ b/drivers/regulator/devres.c @@ -115,6 +115,61 @@ void devm_regulator_put(struct regulator *regulator) } EXPORT_SYMBOL_GPL(devm_regulator_put); +static void devm_regulator_off(struct device *dev, void *res) +{ + regulator_disable(*(struct regulator **)res); +} + +/** + * devm_regulator_enable - Resource managed regulator_enable() + * @regulator: regulator to enable + * + * Managed regulator_enable(). Regulator enabled is automatically + * disabled on driver detach. See regulator_enable() for more + * information. + */ +int devm_regulator_enable(struct device *dev, struct regulator *regulator) +{ + struct regulator **ptr; + int ret; + + ptr = devres_alloc(devm_regulator_off, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return -ENOMEM; + + ret = regulator_enable(regulator); + if (!ret) { + *ptr = regulator; + devres_add(dev, ptr); + } else { + devres_free(ptr); + } + + return ret; +} +EXPORT_SYMBOL_GPL(devm_regulator_enable); + +/** + * devm_regulator_disable - Resource managed regulator_disable() + * @regulator: regulator to disable + * + * Disable a regulator enabled by devm_regulator_enable(). + * Normally this function will not need to be called and the + * resource management code will ensure that the regulator is + * disabled. + */ +void devm_regulator_disable(struct regulator *regulator) +{ + int rc; + + rc = devres_release(regulator->dev, devm_regulator_off, + devm_regulator_match, regulator); + + if (rc != 0) + WARN_ON(rc); +} +EXPORT_SYMBOL_GPL(devm_regulator_disable); + struct regulator_bulk_devres { struct regulator_bulk_data *consumers; int num_consumers; -- 2.20.1
Re: [PATCH 3/3] xen/blkback: Use refcount_t for refcount
On Thu, Aug 8, 2019 at 9:35 PM Roger Pau Monné wrote: > > On Thu, Aug 08, 2019 at 09:11:00PM +0800, Chuhong Yuan wrote: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > Thanks! > > I think there are more reference counters in blkback than > the one you fixed. There's also an inflight field in xen_blkif_ring, > and a pendcnt in pending_req which look like possible candidates to > switch to use refcount_t, have you looked into switching those two > also? > I will switch those two in next version. > Roger.
[PATCH] iio: adc: max1027: Use device-managed APIs
Use device-managed APIs to simplify the code. Signed-off-by: Chuhong Yuan --- drivers/iio/adc/max1027.c | 30 +++--- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c index da84adfdb819..f1b90c544b82 100644 --- a/drivers/iio/adc/max1027.c +++ b/drivers/iio/adc/max1027.c @@ -427,8 +427,9 @@ static int max1027_probe(struct spi_device *spi) return -ENOMEM; } - ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, -&max1027_trigger_handler, NULL); + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, + &iio_pollfunc_store_time, + &max1027_trigger_handler, NULL); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to setup buffer\n"); return ret; @@ -439,7 +440,7 @@ static int max1027_probe(struct spi_device *spi) if (st->trig == NULL) { ret = -ENOMEM; dev_err(&indio_dev->dev, "Failed to allocate iio trigger\n"); - goto fail_trigger_alloc; + return ret; } st->trig->ops = &max1027_trigger_ops; @@ -454,7 +455,7 @@ static int max1027_probe(struct spi_device *spi) spi->dev.driver->name, st->trig); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to allocate IRQ.\n"); - goto fail_dev_register; + return ret; } /* Disable averaging */ @@ -462,33 +463,16 @@ static int max1027_probe(struct spi_device *spi) ret = spi_write(st->spi, &st->reg, 1); if (ret < 0) { dev_err(&indio_dev->dev, "Failed to configure averaging register\n"); - goto fail_dev_register; - } - - ret = iio_device_register(indio_dev); - if (ret < 0) { - dev_err(&indio_dev->dev, "Failed to register iio device\n"); - goto fail_dev_register; + return ret; } - return 0; - -fail_dev_register: -fail_trigger_alloc: - iio_triggered_buffer_cleanup(indio_dev); - - return ret; + return devm_iio_device_register(&spi->dev, indio_dev); } static int max1027_remove(struct spi_device *spi) { - struct iio_dev *indio_dev = spi_get_drvdata(spi); - pr_debug("%s: remove(spi = 0x%p)\n", __func__, spi); - iio_device_unregister(indio_dev); - iio_triggered_buffer_cleanup(indio_dev); - return 0; } -- 2.20.1
[PATCH] crypto: cryptd - Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- crypto/cryptd.c | 44 ++-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/crypto/cryptd.c b/crypto/cryptd.c index 3748f9b4516d..927760b316a4 100644 --- a/crypto/cryptd.c +++ b/crypto/cryptd.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include #include @@ -63,7 +63,7 @@ struct aead_instance_ctx { }; struct cryptd_skcipher_ctx { - atomic_t refcnt; + refcount_t refcnt; struct crypto_sync_skcipher *child; }; @@ -72,7 +72,7 @@ struct cryptd_skcipher_request_ctx { }; struct cryptd_hash_ctx { - atomic_t refcnt; + refcount_t refcnt; struct crypto_shash *child; }; @@ -82,7 +82,7 @@ struct cryptd_hash_request_ctx { }; struct cryptd_aead_ctx { - atomic_t refcnt; + refcount_t refcnt; struct crypto_aead *child; }; @@ -127,7 +127,7 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue, { int cpu, err; struct cryptd_cpu_queue *cpu_queue; - atomic_t *refcnt; + refcount_t *refcnt; cpu = get_cpu(); cpu_queue = this_cpu_ptr(queue->cpu_queue); @@ -140,10 +140,10 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue, queue_work_on(cpu, cryptd_wq, &cpu_queue->work); - if (!atomic_read(refcnt)) + if (!refcount_read(refcnt)) goto out_put_cpu; - atomic_inc(refcnt); + refcount_inc(refcnt); out_put_cpu: put_cpu(); @@ -270,13 +270,13 @@ static void cryptd_skcipher_complete(struct skcipher_request *req, int err) struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req); struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm); struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req); - int refcnt = atomic_read(&ctx->refcnt); + int refcnt = refcount_read(&ctx->refcnt); local_bh_disable(); rctx->complete(&req->base, err); local_bh_enable(); - if (err != -EINPROGRESS && refcnt && atomic_dec_and_test(&ctx->refcnt)) + if (err != -EINPROGRESS && refcnt && refcount_dec_and_test(&ctx->refcnt)) crypto_free_skcipher(tfm); } @@ -521,13 +521,13 @@ static void cryptd_hash_complete(struct ahash_request *req, int err) struct crypto_ahash *tfm = crypto_ahash_reqtfm(req); struct cryptd_hash_ctx *ctx = crypto_ahash_ctx(tfm); struct cryptd_hash_request_ctx *rctx = ahash_request_ctx(req); - int refcnt = atomic_read(&ctx->refcnt); + int refcnt = refcount_read(&ctx->refcnt); local_bh_disable(); rctx->complete(&req->base, err); local_bh_enable(); - if (err != -EINPROGRESS && refcnt && atomic_dec_and_test(&ctx->refcnt)) + if (err != -EINPROGRESS && refcnt && refcount_dec_and_test(&ctx->refcnt)) crypto_free_ahash(tfm); } @@ -772,13 +772,13 @@ static void cryptd_aead_crypt(struct aead_request *req, out: ctx = crypto_aead_ctx(tfm); - refcnt = atomic_read(&ctx->refcnt); + refcnt = refcount_read(&ctx->refcnt); local_bh_disable(); compl(&req->base, err); local_bh_enable(); - if (err != -EINPROGRESS && refcnt && atomic_dec_and_test(&ctx->refcnt)) + if (err != -EINPROGRESS && refcnt && refcount_dec_and_test(&ctx->refcnt)) crypto_free_aead(tfm); } @@ -979,7 +979,7 @@ struct cryptd_skcipher *cryptd_alloc_skcipher(const char *alg_name, } ctx = crypto_skcipher_ctx(tfm); - atomic_set(&ctx->refcnt, 1); + refcount_set(&ctx->refcnt, 1); return container_of(tfm, struct cryptd_skcipher, base); } @@ -997,7 +997,7 @@ bool cryptd_skcipher_queued(struct cryptd_skcipher *tfm) { struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(&tfm->base); - return atomic_read(&ctx->refcnt) - 1; + return refcount_read(&ctx->refcnt) - 1; } EXPORT_SYMBOL_GPL(cryptd_skcipher_queued); @@ -1005,7 +1005,7 @@ void cryptd_free_skcipher(struct cryptd_skcipher *tfm) { struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(&tfm->base); - if (atomic_dec_and_test(&ctx->refcnt)) + if (refcount_dec_and_test(&ctx->refcnt)) crypto_free_skcipher(&tfm->base); } EXPORT_SYMBOL_GPL(cryptd_free_skcipher); @@ -1029,7 +1029,7 @@ struct cryptd_ahash *cryptd_alloc_ahash(cons
[PATCH] powerpc/mm: Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- arch/powerpc/mm/book3s64/mmu_context.c | 2 +- arch/powerpc/mm/book3s64/pgtable.c | 7 +++ arch/powerpc/mm/pgtable-frag.c | 9 - include/linux/mm_types.h | 3 ++- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c index 2d0cb5ba9a47..f836fd5a6abc 100644 --- a/arch/powerpc/mm/book3s64/mmu_context.c +++ b/arch/powerpc/mm/book3s64/mmu_context.c @@ -231,7 +231,7 @@ static void pmd_frag_destroy(void *pmd_frag) /* drop all the pending references */ count = ((unsigned long)pmd_frag & ~PAGE_MASK) >> PMD_FRAG_SIZE_SHIFT; /* We allow PTE_FRAG_NR fragments from a PTE page */ - if (atomic_sub_and_test(PMD_FRAG_NR - count, &page->pt_frag_refcount)) { + if (refcount_sub_and_test(PMD_FRAG_NR - count, &page->pt_frag_refcount)) { pgtable_pmd_page_dtor(page); __free_page(page); } diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c index 7d0e0d0d22c4..40056896ce4e 100644 --- a/arch/powerpc/mm/book3s64/pgtable.c +++ b/arch/powerpc/mm/book3s64/pgtable.c @@ -277,7 +277,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm) return NULL; } - atomic_set(&page->pt_frag_refcount, 1); + refcount_set(&page->pt_frag_refcount, 1); ret = page_address(page); /* @@ -294,7 +294,7 @@ static pmd_t *__alloc_for_pmdcache(struct mm_struct *mm) * count. */ if (likely(!mm->context.pmd_frag)) { - atomic_set(&page->pt_frag_refcount, PMD_FRAG_NR); + refcount_set(&page->pt_frag_refcount, PMD_FRAG_NR); mm->context.pmd_frag = ret + PMD_FRAG_SIZE; } spin_unlock(&mm->page_table_lock); @@ -317,8 +317,7 @@ void pmd_fragment_free(unsigned long *pmd) { struct page *page = virt_to_page(pmd); - BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0); - if (atomic_dec_and_test(&page->pt_frag_refcount)) { + if (refcount_dec_and_test(&page->pt_frag_refcount)) { pgtable_pmd_page_dtor(page); __free_page(page); } diff --git a/arch/powerpc/mm/pgtable-frag.c b/arch/powerpc/mm/pgtable-frag.c index a7b05214760c..4ef8231b677f 100644 --- a/arch/powerpc/mm/pgtable-frag.c +++ b/arch/powerpc/mm/pgtable-frag.c @@ -24,7 +24,7 @@ void pte_frag_destroy(void *pte_frag) /* drop all the pending references */ count = ((unsigned long)pte_frag & ~PAGE_MASK) >> PTE_FRAG_SIZE_SHIFT; /* We allow PTE_FRAG_NR fragments from a PTE page */ - if (atomic_sub_and_test(PTE_FRAG_NR - count, &page->pt_frag_refcount)) { + if (refcount_sub_and_test(PTE_FRAG_NR - count, &page->pt_frag_refcount)) { pgtable_page_dtor(page); __free_page(page); } @@ -71,7 +71,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, int kernel) return NULL; } - atomic_set(&page->pt_frag_refcount, 1); + refcount_set(&page->pt_frag_refcount, 1); ret = page_address(page); /* @@ -87,7 +87,7 @@ static pte_t *__alloc_for_ptecache(struct mm_struct *mm, int kernel) * count. */ if (likely(!pte_frag_get(&mm->context))) { - atomic_set(&page->pt_frag_refcount, PTE_FRAG_NR); + refcount_set(&page->pt_frag_refcount, PTE_FRAG_NR); pte_frag_set(&mm->context, ret + PTE_FRAG_SIZE); } spin_unlock(&mm->page_table_lock); @@ -110,8 +110,7 @@ void pte_fragment_free(unsigned long *table, int kernel) { struct page *page = virt_to_page(table); - BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0); - if (atomic_dec_and_test(&page->pt_frag_refcount)) { + if (refcount_dec_and_test(&page->pt_frag_refcount)) { if (!kernel) pgtable_page_dtor(page); __free_page(page); diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 3a37a89eb7a7..7fe23a3faf95 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -14,6 +14,7 @@ #include #include #include +#include #include @@ -147,7 +148,7 @@ struct page { unsigned long _pt_pad_2;/* mapping */ union { struct mm_struct *pt_mm; /* x86 pgds only */ -
[PATCH 2/2] s390/mm: Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- arch/s390/include/asm/gmap.h | 4 +++- arch/s390/mm/gmap.c | 10 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h index fcbd638fb9f4..37f96b6f0e61 100644 --- a/arch/s390/include/asm/gmap.h +++ b/arch/s390/include/asm/gmap.h @@ -9,6 +9,8 @@ #ifndef _ASM_S390_GMAP_H #define _ASM_S390_GMAP_H +#include + /* Generic bits for GMAP notification on DAT table entry changes. */ #define GMAP_NOTIFY_SHADOW 0x2 #define GMAP_NOTIFY_MPROT 0x1 @@ -46,7 +48,7 @@ struct gmap { struct radix_tree_root guest_to_host; struct radix_tree_root host_to_guest; spinlock_t guest_table_lock; - atomic_t ref_count; + refcount_t ref_count; unsigned long *table; unsigned long asce; unsigned long asce_end; diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index 39c3a6e3d262..cd8e03f04d6d 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -67,7 +67,7 @@ static struct gmap *gmap_alloc(unsigned long limit) INIT_RADIX_TREE(&gmap->host_to_rmap, GFP_ATOMIC); spin_lock_init(&gmap->guest_table_lock); spin_lock_init(&gmap->shadow_lock); - atomic_set(&gmap->ref_count, 1); + refcount_set(&gmap->ref_count, 1); page = alloc_pages(GFP_KERNEL, CRST_ALLOC_ORDER); if (!page) goto out_free; @@ -214,7 +214,7 @@ static void gmap_free(struct gmap *gmap) */ struct gmap *gmap_get(struct gmap *gmap) { - atomic_inc(&gmap->ref_count); + refcount_inc(&gmap->ref_count); return gmap; } EXPORT_SYMBOL_GPL(gmap_get); @@ -227,7 +227,7 @@ EXPORT_SYMBOL_GPL(gmap_get); */ void gmap_put(struct gmap *gmap) { - if (atomic_dec_return(&gmap->ref_count) == 0) + if (refcount_dec_and_test(&gmap->ref_count)) gmap_free(gmap); } EXPORT_SYMBOL_GPL(gmap_put); @@ -1594,7 +1594,7 @@ static struct gmap *gmap_find_shadow(struct gmap *parent, unsigned long asce, continue; if (!sg->initialized) return ERR_PTR(-EAGAIN); - atomic_inc(&sg->ref_count); + refcount_inc(&sg->ref_count); return sg; } return NULL; @@ -1682,7 +1682,7 @@ struct gmap *gmap_shadow(struct gmap *parent, unsigned long asce, } } } - atomic_set(&new->ref_count, 2); + refcount_set(&new->ref_count, 2); list_add(&new->list, &parent->children); if (asce & _ASCE_REAL_SPACE) { /* nothing to protect, return right away */ -- 2.20.1
[PATCH 1/2] s390/extmem: Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- arch/s390/mm/extmem.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/arch/s390/mm/extmem.c b/arch/s390/mm/extmem.c index 0b5622714c12..78e801287c72 100644 --- a/arch/s390/mm/extmem.c +++ b/arch/s390/mm/extmem.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -64,7 +65,7 @@ struct dcss_segment { char res_name[16]; unsigned long start_addr; unsigned long end; - atomic_t ref_count; + refcount_t ref_count; int do_nonshared; unsigned int vm_segtype; struct qrange range[6]; @@ -362,7 +363,7 @@ __segment_load (char *name, int do_nonshared, unsigned long *addr, unsigned long seg->start_addr = start_addr; seg->end = end_addr; seg->do_nonshared = do_nonshared; - atomic_set(&seg->ref_count, 1); + refcount_set(&seg->ref_count, 1); list_add(&seg->list, &dcss_list); *addr = seg->start_addr; *end = seg->end; @@ -422,7 +423,7 @@ segment_load (char *name, int do_nonshared, unsigned long *addr, rc = __segment_load (name, do_nonshared, addr, end); else { if (do_nonshared == seg->do_nonshared) { - atomic_inc(&seg->ref_count); + refcount_inc(&seg->ref_count); *addr = seg->start_addr; *end = seg->end; rc= seg->vm_segtype; @@ -468,7 +469,7 @@ segment_modify_shared (char *name, int do_nonshared) rc = 0; goto out_unlock; } - if (atomic_read (&seg->ref_count) != 1) { + if (refcount_read(&seg->ref_count) != 1) { pr_warn("DCSS %s is in use and cannot be reloaded\n", name); rc = -EAGAIN; goto out_unlock; @@ -544,7 +545,7 @@ segment_unload(char *name) pr_err("Unloading unknown DCSS %s failed\n", name); goto out_unlock; } - if (atomic_dec_return(&seg->ref_count) != 0) + if (!refcount_dec_and_test(&seg->ref_count)) goto out_unlock; release_resource(seg->res); kfree(seg->res); -- 2.20.1
[PATCH 1/2] cxgb4: smt: Add lock for atomic_dec_and_test
The atomic_dec_and_test() is not safe because it is outside of locks. Move the locks of t4_smte_free() to its caller, cxgb4_smt_release() to protect the atomic decrement. Fixes: 3bdb376e6944 ("cxgb4: introduce SMT ops to prepare for SMAC rewrite support") Signed-off-by: Chuhong Yuan --- drivers/net/ethernet/chelsio/cxgb4/smt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.c b/drivers/net/ethernet/chelsio/cxgb4/smt.c index eaf1fb74689c..d6e84c8b5554 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/smt.c +++ b/drivers/net/ethernet/chelsio/cxgb4/smt.c @@ -97,11 +97,9 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac) static void t4_smte_free(struct smt_entry *e) { - spin_lock_bh(&e->lock); if (atomic_read(&e->refcnt) == 0) { /* hasn't been recycled */ e->state = SMT_STATE_UNUSED; } - spin_unlock_bh(&e->lock); } /** @@ -111,8 +109,10 @@ static void t4_smte_free(struct smt_entry *e) */ void cxgb4_smt_release(struct smt_entry *e) { + spin_lock_bh(&e->lock); if (atomic_dec_and_test(&e->refcnt)) t4_smte_free(e); + spin_unlock_bh(&e->lock); } EXPORT_SYMBOL(cxgb4_smt_release); -- 2.20.1
[PATCH 2/2] cxgb4: smt: Use normal int for refcount
All refcount operations are protected by spinlocks now. Then the atomic counter can be replaced by a normal int. This patch depends on PATCH 1/2. Signed-off-by: Chuhong Yuan --- drivers/net/ethernet/chelsio/cxgb4/smt.c | 14 +++--- drivers/net/ethernet/chelsio/cxgb4/smt.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.c b/drivers/net/ethernet/chelsio/cxgb4/smt.c index d6e84c8b5554..01c65d13fc0e 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/smt.c +++ b/drivers/net/ethernet/chelsio/cxgb4/smt.c @@ -57,7 +57,7 @@ struct smt_data *t4_init_smt(void) s->smtab[i].state = SMT_STATE_UNUSED; memset(&s->smtab[i].src_mac, 0, ETH_ALEN); spin_lock_init(&s->smtab[i].lock); - atomic_set(&s->smtab[i].refcnt, 0); + s->smtab[i].refcnt = 0; } return s; } @@ -68,7 +68,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac) struct smt_entry *e, *end; for (e = &s->smtab[0], end = &s->smtab[s->smt_size]; e != end; ++e) { - if (atomic_read(&e->refcnt) == 0) { + if (e->refcnt == 0) { if (!first_free) first_free = e; } else { @@ -97,7 +97,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac) static void t4_smte_free(struct smt_entry *e) { - if (atomic_read(&e->refcnt) == 0) { /* hasn't been recycled */ + if (e->refcnt == 0) { /* hasn't been recycled */ e->state = SMT_STATE_UNUSED; } } @@ -110,7 +110,7 @@ static void t4_smte_free(struct smt_entry *e) void cxgb4_smt_release(struct smt_entry *e) { spin_lock_bh(&e->lock); - if (atomic_dec_and_test(&e->refcnt)) + if ((--e->refcnt) == 0) t4_smte_free(e); spin_unlock_bh(&e->lock); } @@ -215,14 +215,14 @@ static struct smt_entry *t4_smt_alloc_switching(struct adapter *adap, u16 pfvf, e = find_or_alloc_smte(s, smac); if (e) { spin_lock(&e->lock); - if (!atomic_read(&e->refcnt)) { - atomic_set(&e->refcnt, 1); + if (!e->refcnt) { + e->refcnt = 1; e->state = SMT_STATE_SWITCHING; e->pfvf = pfvf; memcpy(e->src_mac, smac, ETH_ALEN); write_smt_entry(adap, e); } else { - atomic_inc(&e->refcnt); + ++e->refcnt; } spin_unlock(&e->lock); } diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.h b/drivers/net/ethernet/chelsio/cxgb4/smt.h index d6c2cc271398..1268d6e93a47 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/smt.h +++ b/drivers/net/ethernet/chelsio/cxgb4/smt.h @@ -59,7 +59,7 @@ struct smt_entry { u16 idx; u16 pfvf; u8 src_mac[ETH_ALEN]; - atomic_t refcnt; + int refcnt; spinlock_t lock;/* protect smt entry add,removal */ }; -- 2.20.1
[PATCH v3] mlx5: Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- Changes in v3: - Merge v2 patches together. drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++--- drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++--- include/linux/mlx5/driver.h | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/infiniband/hw/mlx5/srq_cmd.c b/drivers/infiniband/hw/mlx5/srq_cmd.c index b0d0687c7a68..8fc3630a9d4c 100644 --- a/drivers/infiniband/hw/mlx5/srq_cmd.c +++ b/drivers/infiniband/hw/mlx5/srq_cmd.c @@ -86,7 +86,7 @@ struct mlx5_core_srq *mlx5_cmd_get_srq(struct mlx5_ib_dev *dev, u32 srqn) xa_lock(&table->array); srq = xa_load(&table->array, srqn); if (srq) - atomic_inc(&srq->common.refcount); + refcount_inc(&srq->common.refcount); xa_unlock(&table->array); return srq; @@ -592,7 +592,7 @@ int mlx5_cmd_create_srq(struct mlx5_ib_dev *dev, struct mlx5_core_srq *srq, if (err) return err; - atomic_set(&srq->common.refcount, 1); + refcount_set(&srq->common.refcount, 1); init_completion(&srq->common.free); err = xa_err(xa_store_irq(&table->array, srq->srqn, srq, GFP_KERNEL)); @@ -675,7 +675,7 @@ static int srq_event_notifier(struct notifier_block *nb, xa_lock(&table->array); srq = xa_load(&table->array, srqn); if (srq) - atomic_inc(&srq->common.refcount); + refcount_inc(&srq->common.refcount); xa_unlock(&table->array); if (!srq) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c index b8ba74de9555..7b44d1e49604 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/qp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c @@ -53,7 +53,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn) common = radix_tree_lookup(&table->tree, rsn); if (common) - atomic_inc(&common->refcount); + refcount_inc(&common->refcount); spin_unlock_irqrestore(&table->lock, flags); @@ -62,7 +62,7 @@ mlx5_get_rsc(struct mlx5_qp_table *table, u32 rsn) void mlx5_core_put_rsc(struct mlx5_core_rsc_common *common) { - if (atomic_dec_and_test(&common->refcount)) + if (refcount_dec_and_test(&common->refcount)) complete(&common->free); } @@ -209,7 +209,7 @@ static int create_resource_common(struct mlx5_core_dev *dev, if (err) return err; - atomic_set(&qp->common.refcount, 1); + refcount_set(&qp->common.refcount, 1); init_completion(&qp->common.free); qp->pid = current->pid; diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h index 0e6da1840c7d..5b56f343ce87 100644 --- a/include/linux/mlx5/driver.h +++ b/include/linux/mlx5/driver.h @@ -47,6 +47,7 @@ #include #include #include +#include #include #include @@ -398,7 +399,7 @@ enum mlx5_res_type { struct mlx5_core_rsc_common { enum mlx5_res_type res; - atomic_trefcount; + refcount_t refcount; struct completion free; }; -- 2.20.1
[PATCH v3 7/8] userns: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/user_namespace.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 8eadadc478f9..e231e902df8a 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -1138,6 +1138,7 @@ ssize_t proc_setgroups_write(struct file *file, const char __user *buf, char kbuf[8], *pos; bool setgroups_allowed; ssize_t ret; + size_t len; /* Only allow a very narrow range of strings to be written */ ret = -EINVAL; @@ -1153,12 +1154,11 @@ ssize_t proc_setgroups_write(struct file *file, const char __user *buf, /* What is being requested? */ ret = -EINVAL; - if (strncmp(pos, "allow", 5) == 0) { - pos += 5; + if ((len = str_has_prefix(pos, "allow"))) { + pos += len; setgroups_allowed = true; - } - else if (strncmp(pos, "deny", 4) == 0) { - pos += 4; + } else if ((len = str_has_prefix(pos, "deny"))) { + pos += len; setgroups_allowed = false; } else -- 2.20.1
[PATCH v3 8/8] watchdog: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/watchdog.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 7f9e7b9306fe..ac7a4b5f856e 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -70,13 +70,13 @@ void __init hardlockup_detector_disable(void) static int __init hardlockup_panic_setup(char *str) { - if (!strncmp(str, "panic", 5)) + if (str_has_prefix(str, "panic")) hardlockup_panic = 1; - else if (!strncmp(str, "nopanic", 7)) + else if (str_has_prefix(str, "nopanic")) hardlockup_panic = 0; - else if (!strncmp(str, "0", 1)) + else if (str_has_prefix(str, "0")) nmi_watchdog_user_enabled = 0; - else if (!strncmp(str, "1", 1)) + else if (str_has_prefix(str, "1")) nmi_watchdog_user_enabled = 1; return 1; } -- 2.20.1
[PATCH v3 5/8] reboot: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/reboot.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/reboot.c b/kernel/reboot.c index c4d472b7f1b4..addb52391177 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -520,6 +520,8 @@ EXPORT_SYMBOL_GPL(orderly_reboot); static int __init reboot_setup(char *str) { + size_t len; + for (;;) { enum reboot_mode *mode; @@ -530,9 +532,9 @@ static int __init reboot_setup(char *str) */ reboot_default = 0; - if (!strncmp(str, "panic_", 6)) { + if ((len = str_has_prefix(str, "panic_"))) { mode = &panic_reboot_mode; - str += 6; + str += len; } else { mode = &reboot_mode; } -- 2.20.1
[PATCH v3 6/8] sched: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/sched/debug.c | 5 +++-- kernel/sched/isolation.c | 9 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index f7e4579e746c..43983ff325b7 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -102,10 +102,11 @@ static int sched_feat_set(char *cmp) { int i; int neg = 0; + size_t len; - if (strncmp(cmp, "NO_", 3) == 0) { + if ((len = str_has_prefix(cmp, "NO_"))) { neg = 1; - cmp += 3; + cmp += len; } i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp); diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c index ccb28085b114..f528bb5996f4 100644 --- a/kernel/sched/isolation.c +++ b/kernel/sched/isolation.c @@ -141,16 +141,17 @@ __setup("nohz_full=", housekeeping_nohz_full_setup); static int __init housekeeping_isolcpus_setup(char *str) { unsigned int flags = 0; + size_t len; while (isalpha(*str)) { - if (!strncmp(str, "nohz,", 5)) { - str += 5; + if ((len = str_has_prefix(str, "nohz,"))) { + str += len; flags |= HK_FLAG_TICK; continue; } - if (!strncmp(str, "domain,", 7)) { - str += 7; + if ((len = str_has_prefix(str, "domain,"))) { + str += len; flags |= HK_FLAG_DOMAIN; continue; } -- 2.20.1
[PATCH v3 2/8] module: Replace strncmp with str_has_prefix
strncmp(str, const, len) is error-prone because len is easy to have typo. The example is the hard-coded len has counting error or sizeof(const) forgets - 1. So we prefer using newly introduced str_has_prefix() to substitute such strncmp to make code better. Signed-off-by: Chuhong Yuan --- Changes in v3: - Revise the description. kernel/module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index 5933395af9a0..7defa2a4a701 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2251,7 +2251,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info) switch (sym[i].st_shndx) { case SHN_COMMON: /* Ignore common symbols */ - if (!strncmp(name, "__gnu_lto", 9)) + if (str_has_prefix(name, "__gnu_lto")) break; /* We compiled with -fno-common. These are not -- 2.20.1