linux-kernel - Re: [PATCH v3] drm/hyperv: Added error message for fb size greater than allocated

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]

Date:   Mon, 11 Apr 2022 00:55:55 -0700
From:   Saurabh Singh Sengar <ssengar@...ux.microsoft.com>
To:     Dexuan Cui <decui@...rosoft.com>
Cc:     Saurabh Singh Sengar <ssengar@...rosoft.com>,
        "drawat.floss@...il.com" <drawat.floss@...il.com>,
        "airlied@...ux.ie" <airlied@...ux.ie>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
Subject: Re: [PATCH v3] drm/hyperv: Added error message for fb size greater
 than allocated

On Mon, Apr 11, 2022 at 06:40:38AM +0000, Dexuan Cui wrote:
> > Subject: [PATCH v3] drm/hyperv: Added error message for fb size greater than
> > allocated
> > 
> > Added error message when the size of requested framebuffer is more than
> > the allocated size by vmbus mmio region for framebuffer
> 
> "Added" --> "Add"? My impression is that we don't use past tense in the 

Ok.

> Subject and the commit message. See
> "git log drivers/gpu/drm/hyperv/hyperv_drm_modeset.c".
>  
> > --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> > @@ -123,8 +123,11 @@ static int hyperv_pipe_check(struct
> > drm_simple_display_pipe *pipe,
> >  	if (fb->format->format != DRM_FORMAT_XRGB8888)
> >  		return -EINVAL;
> > 
> > -	if (fb->pitches[0] * fb->height > hv->fb_size)
> > +	if (fb->pitches[0] * fb->height > hv->fb_size) {
> > +		drm_err(&hv->dev, "hv->hdev, fb size requested by process %s
> > for %d X %d (pitch %d) is greater than allocated size %ld\n",
> Should we use drm_err_ratelimited() instead of drm_err()?

The error is not produced in good cases, for every resolution change which is violating this error should print once.
I suggest having it print every time some application tries to violate the policy set at boot time.
If we use ratelimit many resolutions error change will be suppressed. Please let me know your thoughts.


> 
> The line exceeds 80 chars.

At first I tried braking the line to respect 80 character boundary, but checkpatch.pl reported it as a problem.
And these new lines are suggested by checkpatch.pl itself.
Looks the recent rule realted to 80 charachters are changed. Ref : https://www.theregister.com/2020/06/01/linux_5_7/#:~:text=Linux%20kernel%20overlord%20Linus%20Torvalds,the%20topic%20of%20line%20lengths.

> 
> > +		current->comm, fb->width, fb->height, fb->pitches[0], hv->fb_size);
> >  		return -EINVAL;
> > +	}
> 
> Maybe we can use the below:
> 	drm_err_ratelimited(&hv->dev, "%s: requested %dX%d (pitch %d) "
>                      "exceeds fb_size %ld\n",
>                      current->comm, fb->width, fb->height,
>                      fb->pitches[0], hv->fb_size);
> 
> Note: the first chars of last 3 lines should align with the "&" in the
> same column. Please run "scripts/checkpatch.pl" against the patch.

I have tested checkpatch.pl before sending, for the current patch there is no problem from checkpatch.pl