| Message ID | 20240927153305.27642-1-liankun.yang@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] drm/mediatek: Fix potential KP on 0 bytes nvmem cell read | expand |
On Fri, 27 Sep 2024 11:03, Liankun Yang <liankun.yang@mediatek.com> wrote: >If the len is 0, kernel crash will occur when performing operations. >Add the len check conditions to prevent kernel crash. > >Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver") >Signed-off-by: Liankun Yang <liankun.yang@mediatek.com> >--- >Changes in V2: >- Fix the commit title. >- Remove blank line between the Fixes and Signe-off-by. >- Modify the judgment writing method. >Per suggestion from the previous thread: >https://patchwork.kernel.org/project/linux-mediatek/patch/20240925084116.28848-1-liankun.yang@mediatek.com/ >--- > drivers/gpu/drm/mediatek/mtk_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c >index d8796a904eca..9ecdf62398ee 100644 >--- a/drivers/gpu/drm/mediatek/mtk_dp.c >+++ b/drivers/gpu/drm/mediatek/mtk_dp.c >@@ -1082,7 +1082,7 @@ static void mtk_dp_get_calibration_data(struct mtk_dp *mtk_dp) > buf = (u32 *)nvmem_cell_read(cell, &len); > nvmem_cell_put(cell); > >- if (IS_ERR(buf) || ((len / sizeof(u32)) != 4)) { >+ if (IS_ERR(buf) || !len || ((len / sizeof(u32)) != 4)) { > dev_warn(dev, "Failed to read nvmem_cell_read\n"); Hello Liankun, Would you be able to describe the crash with more details? I'm afraid I don't understand the fix? if len is 0, dividing 0 by sizeof(u32) is 0 and thus != 4 So to me, checking for len != 0 is redundant? Thx, Guillaume. > > if (!IS_ERR(buf)) >-- >2.45.2
Hi, Liankun: On Fri, 2024-09-27 at 17:03 +0800, Liankun Yang wrote: > If the len is 0, kernel crash will occur when performing operations. > Add the len check conditions to prevent kernel crash. > > Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver") > Signed-off-by: Liankun Yang <liankun.yang@mediatek.com> > --- > Changes in V2: > - Fix the commit title. > - Remove blank line between the Fixes and Signe-off-by. > - Modify the judgment writing method. > Per suggestion from the previous thread: > https://patchwork.kernel.org/project/linux-mediatek/patch/20240925084116.28848-1-liankun.yang@mediatek.com/ > --- > drivers/gpu/drm/mediatek/mtk_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c > index d8796a904eca..9ecdf62398ee 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dp.c > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c > @@ -1082,7 +1082,7 @@ static void mtk_dp_get_calibration_data(struct mtk_dp *mtk_dp) > buf = (u32 *)nvmem_cell_read(cell, &len); > nvmem_cell_put(cell); > > - if (IS_ERR(buf) || ((len / sizeof(u32)) != 4)) { In original code, when len = 0, no kernel panic occur. Nacked by me. Regards, CK > + if (IS_ERR(buf) || !len || ((len / sizeof(u32)) != 4)) { > dev_warn(dev, "Failed to read nvmem_cell_read\n"); > > if (!IS_ERR(buf))
On Wed, 2024-10-09 at 08:41 -0700, Guillaume Ranquet wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On Fri, 27 Sep 2024 11:03, Liankun Yang <liankun.yang@mediatek.com> > wrote: > >If the len is 0, kernel crash will occur when performing operations. > >Add the len check conditions to prevent kernel crash. > > > >Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort > driver") > >Signed-off-by: Liankun Yang <liankun.yang@mediatek.com> > >--- > >Changes in V2: > >- Fix the commit title. > >- Remove blank line between the Fixes and Signe-off-by. > >- Modify the judgment writing method. > >Per suggestion from the previous thread: > > > https://patchwork.kernel.org/project/linux-mediatek/patch/20240925084116.28848-1-liankun.yang@mediatek.com/ > >--- > > drivers/gpu/drm/mediatek/mtk_dp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c > b/drivers/gpu/drm/mediatek/mtk_dp.c > >index d8796a904eca..9ecdf62398ee 100644 > >--- a/drivers/gpu/drm/mediatek/mtk_dp.c > >+++ b/drivers/gpu/drm/mediatek/mtk_dp.c > >@@ -1082,7 +1082,7 @@ static void mtk_dp_get_calibration_data(struct > mtk_dp *mtk_dp) > > buf = (u32 *)nvmem_cell_read(cell, &len); > > nvmem_cell_put(cell); > > > >-if (IS_ERR(buf) || ((len / sizeof(u32)) != 4)) { > >+if (IS_ERR(buf) || !len || ((len / sizeof(u32)) != 4)) { > > dev_warn(dev, "Failed to read nvmem_cell_read\n"); > > Hello Liankun, > Would you be able to describe the crash with more details? > > I'm afraid I don't understand the fix? > > if len is 0, dividing 0 by sizeof(u32) is 0 and thus != 4 > So to me, checking for len != 0 is redundant? > > Thx, > Guillaume. Hello Guillaume, Thank you for your correction. The check for len != 0 is redundant. I did not review this patch carefully, and will review it again next time. Thx, Liankun. > > > > if (!IS_ERR(buf)) > >-- > >2.45.2
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c index d8796a904eca..9ecdf62398ee 100644 --- a/drivers/gpu/drm/mediatek/mtk_dp.c +++ b/drivers/gpu/drm/mediatek/mtk_dp.c @@ -1082,7 +1082,7 @@ static void mtk_dp_get_calibration_data(struct mtk_dp *mtk_dp) buf = (u32 *)nvmem_cell_read(cell, &len); nvmem_cell_put(cell); - if (IS_ERR(buf) || ((len / sizeof(u32)) != 4)) { + if (IS_ERR(buf) || !len || ((len / sizeof(u32)) != 4)) { dev_warn(dev, "Failed to read nvmem_cell_read\n"); if (!IS_ERR(buf))
If the len is 0, kernel crash will occur when performing operations. Add the len check conditions to prevent kernel crash. Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver") Signed-off-by: Liankun Yang <liankun.yang@mediatek.com> --- Changes in V2: - Fix the commit title. - Remove blank line between the Fixes and Signe-off-by. - Modify the judgment writing method. Per suggestion from the previous thread: https://patchwork.kernel.org/project/linux-mediatek/patch/20240925084116.28848-1-liankun.yang@mediatek.com/ --- drivers/gpu/drm/mediatek/mtk_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)