linux-kernel - Re: [PATCH] omapfb/dss: Delete an error message for a failed memory allocation in three functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1511809633.32426.70.camel@perches.com>
Date:   Mon, 27 Nov 2017 11:07:13 -0800
From:   Joe Perches <joe@...ches.com>
To:     "Andrew F. Davis" <afd@...com>,
        SF Markus Elfring <elfring@...rs.sourceforge.net>,
        linux-omap@...r.kernel.org, linux-fbdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org,
        Arvind Yadav <arvind.yadav.cs@...il.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Tomi Valkeinen <tomi.valkeinen@...com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] omapfb/dss: Delete an error message for a failed memory
 allocation in three functions

On Mon, 2017-11-27 at 10:43 -0600, Andrew F. Davis wrote:
> On 11/26/2017 12:55 PM, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@...rs.sourceforge.net>
> > Date: Sun, 26 Nov 2017 19:46:09 +0100
> > 
> > Omit an extra message for a memory allocation failure in these functions.
> > 
> > This issue was detected by using the Coccinelle software.
> > 
> > Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>
> > ---
> 
> nak, unlike many others, these message give extra info on which
> allocation failed, that can be useful.

<shrug>  Not really.  There are tradeoffs.

There is the generic stack dump on OOM so the module/line
is already known.

The existence of these messages increases code size which
also make the OOM condition slightly more likely.

These are generally used only at initialization and those
if you are OOM at initialization, bad things happen anyway
so where the specific OOM occurred doesn't really matter.

Markus' commit messages are always really poor descriptions
of why these removals are somewhat useful and the commit
could/should/might be applied.

Your choice.

> >  drivers/video/fbdev/omap2/omapfb/dss/dispc.c    | 4 +---
> >  drivers/video/fbdev/omap2/omapfb/dss/dss.c      | 4 +---
> >  drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 4 +---
> >  3 files changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > index 7a75dfda9845..10164a3bae4a 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> > @@ -3982,10 +3982,8 @@ static int dispc_init_features(struct platform_device *pdev)
> >  	struct dispc_features *dst;
> >  
> >  	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -	if (!dst) {
> > -		dev_err(&pdev->dev, "Failed to allocate DISPC Features\n");
> > +	if (!dst)
> >  		return -ENOMEM;
> > -	}
> >  
> >  	switch (omapdss_get_version()) {
> >  	case OMAPDSS_VER_OMAP24xx:
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > index 48c6500c24e1..a5de13777e2b 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> > @@ -893,10 +893,8 @@ static int dss_init_features(struct platform_device *pdev)
> >  	struct dss_features *dst;
> >  
> >  	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -	if (!dst) {
> > -		dev_err(&pdev->dev, "Failed to allocate local DSS Features\n");
> > +	if (!dst)
> >  		return -ENOMEM;
> > -	}
> >  
> >  	switch (omapdss_get_version()) {
> >  	case OMAPDSS_VER_OMAP24xx:
> > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > index 9a13c35fd6d8..d25eea10c665 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c
> > @@ -195,10 +195,8 @@ static int hdmi_phy_init_features(struct platform_device *pdev)
> >  	const struct hdmi_phy_features *src;
> >  
> >  	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
> > -	if (!dst) {
> > -		dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
> > +	if (!dst)
> >  		return -ENOMEM;
> > -	}
> >  
> >  	switch (omapdss_get_version()) {
> >  	case OMAPDSS_VER_OMAP4430_ES1:
> >