Initialization of the I2C adapter was allowed to fail. The mgag200 driver would have continued without DDC support. Had this happened in practice, it would have led to segmentation faults in the connector code. Resolve this problem by failing driver initialization on I2C- related errors.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_i2c.c | 13 ++++++++----- drivers/gpu/drm/mgag200/mgag200_mode.c | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c b/drivers/gpu/drm/mgag200/mgag200_i2c.c index ac8e34eef5138..31e2c641a7814 100644 --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c @@ -120,7 +120,7 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev)
i2c = kzalloc(sizeof(struct mga_i2c_chan), GFP_KERNEL); if (!i2c) - return NULL; + return ERR_PTR(-ENOMEM);
i2c->data = data; i2c->clock = clock; @@ -142,11 +142,14 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev) i2c->bit.getscl = mga_gpio_getscl;
ret = i2c_bit_add_bus(&i2c->adapter); - if (ret) { - kfree(i2c); - i2c = NULL; - } + if (ret) + goto err_kfree; + return i2c; + +err_kfree: + kfree(i2c); + return ERR_PTR(ret); }
void mgag200_i2c_destroy(struct mga_i2c_chan *i2c) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index abde7655477db..79557ca056f08 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -815,8 +815,10 @@ static int mgag200_vga_connector_init(struct mga_device *mdev) int ret;
i2c = mgag200_i2c_create(dev); - if (!i2c) - drm_warn(dev, "failed to add DDC bus\n"); + if (IS_ERR(i2c)) { + drm_err(dev, "failed to add DDC bus: %d\n", ret); + return PTR_ERR(i2c); + }
ret = drm_connector_init_with_ddc(dev, connector, &mga_vga_connector_funcs,
On 05/05/2022 17:22, Thomas Zimmermann wrote:
Initialization of the I2C adapter was allowed to fail. The mgag200 driver would have continued without DDC support. Had this happened in practice, it would have led to segmentation faults in the connector code. Resolve this problem by failing driver initialization on I2C- related errors.
Reviewed-by: Jocelyn Falempe jfalempe@redhat.com
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/mgag200/mgag200_i2c.c | 13 ++++++++----- drivers/gpu/drm/mgag200/mgag200_mode.c | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c b/drivers/gpu/drm/mgag200/mgag200_i2c.c index ac8e34eef5138..31e2c641a7814 100644 --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c @@ -120,7 +120,7 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev)
i2c = kzalloc(sizeof(struct mga_i2c_chan), GFP_KERNEL); if (!i2c)
return NULL;
return ERR_PTR(-ENOMEM);i2c->data = data; i2c->clock = clock;
@@ -142,11 +142,14 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev) i2c->bit.getscl = mga_gpio_getscl;
ret = i2c_bit_add_bus(&i2c->adapter);
- if (ret) {
kfree(i2c);i2c = NULL;- }
- if (ret)
goto err_kfree;- return i2c;
+err_kfree:
kfree(i2c);
return ERR_PTR(ret); }
void mgag200_i2c_destroy(struct mga_i2c_chan *i2c)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index abde7655477db..79557ca056f08 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -815,8 +815,10 @@ static int mgag200_vga_connector_init(struct mga_device *mdev) int ret;
i2c = mgag200_i2c_create(dev);
- if (!i2c)
drm_warn(dev, "failed to add DDC bus\n");
if (IS_ERR(i2c)) {
drm_err(dev, "failed to add DDC bus: %d\n", ret);return PTR_ERR(i2c);}
ret = drm_connector_init_with_ddc(dev, connector, &mga_vga_connector_funcs,
Hi
Am 05.05.22 um 17:49 schrieb Jocelyn Falempe:
On 05/05/2022 17:22, Thomas Zimmermann wrote:
Initialization of the I2C adapter was allowed to fail. The mgag200 driver would have continued without DDC support. Had this happened in practice, it would have led to segmentation faults in the connector code. Resolve this problem by failing driver initialization on I2C- related errors.
Reviewed-by: Jocelyn Falempe jfalempe@redhat.com
Thanks a lot. I'll fix the reports from the automated tests and resubmit this patch as part of a larger patchset.
Best regards Thomas
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/mgag200/mgag200_i2c.c | 13 ++++++++----- drivers/gpu/drm/mgag200/mgag200_mode.c | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_i2c.c b/drivers/gpu/drm/mgag200/mgag200_i2c.c index ac8e34eef5138..31e2c641a7814 100644 --- a/drivers/gpu/drm/mgag200/mgag200_i2c.c +++ b/drivers/gpu/drm/mgag200/mgag200_i2c.c @@ -120,7 +120,7 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev) i2c = kzalloc(sizeof(struct mga_i2c_chan), GFP_KERNEL); if (!i2c) - return NULL; + return ERR_PTR(-ENOMEM); i2c->data = data; i2c->clock = clock; @@ -142,11 +142,14 @@ struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev) i2c->bit.getscl = mga_gpio_getscl; ret = i2c_bit_add_bus(&i2c->adapter); - if (ret) { - kfree(i2c); - i2c = NULL; - } + if (ret) + goto err_kfree;
return i2c;
+err_kfree: + kfree(i2c); + return ERR_PTR(ret); } void mgag200_i2c_destroy(struct mga_i2c_chan *i2c) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index abde7655477db..79557ca056f08 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -815,8 +815,10 @@ static int mgag200_vga_connector_init(struct mga_device *mdev) int ret; i2c = mgag200_i2c_create(dev); - if (!i2c) - drm_warn(dev, "failed to add DDC bus\n"); + if (IS_ERR(i2c)) { + drm_err(dev, "failed to add DDC bus: %d\n", ret); + return PTR_ERR(i2c); + } ret = drm_connector_init_with_ddc(dev, connector, &mga_vga_connector_funcs,
Hi Thomas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next] [also build test WARNING on drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip tegra-drm/drm/tegra/for-next v5.18-rc5 next-20220505] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-mgag200... base: git://anongit.freedesktop.org/drm/drm drm-next config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220506/202205060439.bfJ2FmzU-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e004fb787698440a387750db7f8028e7cb14cfc) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/11682b9fc557a02edac08b0dedc91c... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Thomas-Zimmermann/drm-mgag200-Fail-on-I2C-initialization-errors/20220505-234643 git checkout 11682b9fc557a02edac08b0dedc91ce704c2f749 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/mgag200/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/mgag200/mgag200_mode.c:819:47: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
drm_err(dev, "failed to add DDC bus: %d\n", ret); ^~~ include/drm/drm_print.h:438:46: note: expanded from macro 'drm_err' __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__) ^~~~~~~~~~~ include/drm/drm_print.h:425:48: note: expanded from macro '__drm_printk' dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__) ^~~~~~~~~~~ include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err' dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~~~~~~~~~ include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap' _p_func(dev, fmt, ##__VA_ARGS__); \ ^~~~~~~~~~~ drivers/gpu/drm/mgag200/mgag200_mode.c:815:9: note: initialize the variable 'ret' to silence this warning int ret; ^ = 0 1 warning generated.
vim +/ret +819 drivers/gpu/drm/mgag200/mgag200_mode.c
808 809 static int mgag200_vga_connector_init(struct mga_device *mdev) 810 { 811 struct drm_device *dev = &mdev->base; 812 struct mga_connector *mconnector = &mdev->connector; 813 struct drm_connector *connector = &mconnector->base; 814 struct mga_i2c_chan *i2c; 815 int ret; 816 817 i2c = mgag200_i2c_create(dev); 818 if (IS_ERR(i2c)) {
819 drm_err(dev, "failed to add DDC bus: %d\n", ret);
820 return PTR_ERR(i2c); 821 } 822 823 ret = drm_connector_init_with_ddc(dev, connector, 824 &mga_vga_connector_funcs, 825 DRM_MODE_CONNECTOR_VGA, 826 &i2c->adapter); 827 if (ret) 828 goto err_mgag200_i2c_destroy; 829 drm_connector_helper_add(connector, &mga_vga_connector_helper_funcs); 830 831 mconnector->i2c = i2c; 832 833 return 0; 834 835 err_mgag200_i2c_destroy: 836 mgag200_i2c_destroy(i2c); 837 return ret; 838 } 839
Hi Thomas,
url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-mgag200... base: git://anongit.freedesktop.org/drm/drm drm-next config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220506/202205061008.eYVQWRSt-lkp@i...) compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com Reported-by: Dan Carpenter dan.carpenter@oracle.com
smatch warnings: drivers/gpu/drm/mgag200/mgag200_mode.c:819 mgag200_vga_connector_init() error: uninitialized symbol 'ret'.
vim +/ret +819 drivers/gpu/drm/mgag200/mgag200_mode.c
81a15b9a65565dc Thomas Zimmermann 2020-05-07 809 static int mgag200_vga_connector_init(struct mga_device *mdev) 414c453106255b1 Dave Airlie 2012-04-17 810 { 832eddf5d8f4d83 Thomas Zimmermann 2020-06-05 811 struct drm_device *dev = &mdev->base; 81a15b9a65565dc Thomas Zimmermann 2020-05-07 812 struct mga_connector *mconnector = &mdev->connector; 81a15b9a65565dc Thomas Zimmermann 2020-05-07 813 struct drm_connector *connector = &mconnector->base; 81a15b9a65565dc Thomas Zimmermann 2020-05-07 814 struct mga_i2c_chan *i2c; 81a15b9a65565dc Thomas Zimmermann 2020-05-07 815 int ret; 414c453106255b1 Dave Airlie 2012-04-17 816 81a15b9a65565dc Thomas Zimmermann 2020-05-07 817 i2c = mgag200_i2c_create(dev); 11682b9fc557a02 Thomas Zimmermann 2022-05-05 818 if (IS_ERR(i2c)) { 11682b9fc557a02 Thomas Zimmermann 2022-05-05 @819 drm_err(dev, "failed to add DDC bus: %d\n", ret); ^^^ Uninitialized
11682b9fc557a02 Thomas Zimmermann 2022-05-05 820 return PTR_ERR(i2c); 11682b9fc557a02 Thomas Zimmermann 2022-05-05 821 } 414c453106255b1 Dave Airlie 2012-04-17 822 81a15b9a65565dc Thomas Zimmermann 2020-05-07 823 ret = drm_connector_init_with_ddc(dev, connector, 9572ae176a10f3b Andrzej Pietrasiewicz 2019-07-26 824 &mga_vga_connector_funcs, 9572ae176a10f3b Andrzej Pietrasiewicz 2019-07-26 825 DRM_MODE_CONNECTOR_VGA, 81a15b9a65565dc Thomas Zimmermann 2020-05-07 826 &i2c->adapter); 81a15b9a65565dc Thomas Zimmermann 2020-05-07 827 if (ret) 81a15b9a65565dc Thomas Zimmermann 2020-05-07 828 goto err_mgag200_i2c_destroy; 414c453106255b1 Dave Airlie 2012-04-17 829 drm_connector_helper_add(connector, &mga_vga_connector_helper_funcs); 414c453106255b1 Dave Airlie 2012-04-17 830 81a15b9a65565dc Thomas Zimmermann 2020-05-07 831 mconnector->i2c = i2c; 3d5a1c5e300483d Egbert Eich 2013-07-17 832 81a15b9a65565dc Thomas Zimmermann 2020-05-07 833 return 0; 81a15b9a65565dc Thomas Zimmermann 2020-05-07 834 81a15b9a65565dc Thomas Zimmermann 2020-05-07 835 err_mgag200_i2c_destroy: 81a15b9a65565dc Thomas Zimmermann 2020-05-07 836 mgag200_i2c_destroy(i2c); 81a15b9a65565dc Thomas Zimmermann 2020-05-07 837 return ret; 414c453106255b1 Dave Airlie 2012-04-17 838 }
dri-devel@lists.freedesktop.org
-
Dan Carpenter -
Jocelyn Falempe -
kernel test robot -
Thomas Zimmermann