linux-kernel - Re: [PATCH] gma500: handling failed allocation

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] [day] [month] [year] [list]

Message-ID: <20160129174602.7818a35a@lxorguk.ukuu.org.uk>
Date:	Fri, 29 Jan 2016 17:46:02 +0000
From:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
To:	Insu Yun <wuninsu@...il.com>
Cc:	patrik.r.jakobsson@...il.com, airlied@...ux.ie,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	taesoo@...ech.edu, yeongjin.jang@...ech.edu, insu@...ech.edu,
	changwoo@...ech.edu
Subject: Re: [PATCH] gma500: handling failed allocation

On Thu, 28 Jan 2016 19:05:16 -0500
Insu Yun <wuninsu@...il.com> wrote:

> Since drm_property_create_range can be failed in memory pressure, 
> it needs to be handled.
> 
> Signed-off-by: Insu Yun <wuninsu@...il.com>
> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
> index cb95765..31085e4 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -683,6 +683,8 @@ static int psb_create_backlight_property(struct drm_device *dev)
>  		return 0;
>  
>  	backlight = drm_property_create_range(dev, 0, "backlight", 0, 100);
> +	if (!backlight)
> +		return -ENOMEM;
>  
>  	dev_priv->backlight_property = backlight;
>  

NAK.

If we fail to create the backlight we are better off continuing than
failing. The user just loses backlight control rather than having no
display at all.

If you check the callers you'll notice that the only caller doesn't even
check the return code anyway so your patch is a no-op. If you are going
to add error checking to anything with a patch please work back through
the call chain and check the effect of the new error return - if any.

A better patch I think would be to just eliminate the function and turn
it into a tiny bit of inlined code.

I'll send a patch to do that shortly.

Alan