Received-SPF: pass (google.com: domain of linux-kernel+bounces-138986-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 10 Apr 2024 18:36:11 +0200
From: Alexey Gladkov <legion@kernel.org>
To: Jiri Slaby <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	LKML <linux-kernel@vger.kernel.org>, kbd@lists.linux.dev,
	linux-api@vger.kernel.org, linux-fbdev@vger.kernel.org,
	linux-serial@vger.kernel.org, Helge Deller <deller@gmx.de>
Subject: Re: [PATCH v4 2/3] VT: Add KDFONTINFO ioctl
Message-ID: <Zha_-zPHCW8iYT_4@example.org>
References: <74ca50e0-61b1-4d4c-85dd-a5d920548c04@kernel.org>
 <cover.1712080158.git.legion@kernel.org>
 <7cd32f988a147d7617742c9e074c753de0c6bc1f.1712080158.git.legion@kernel.org>
 <211f3c45-7064-475b-b9e1-f6adbbba8879@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <211f3c45-7064-475b-b9e1-f6adbbba8879@kernel.org>

On Wed, Apr 03, 2024 at 07:05:14AM +0200, Jiri Slaby wrote:
> First, there was no need to send this v4 so quickly. Provided we have 
> not settled in v3... This makes the review process painful.
> 
> And then:
> 
> On 02. 04. 24, 19:50, Alexey Gladkov wrote:
> > Each driver has its own restrictions on font size. There is currently no
> > way to understand what the requirements are. The new ioctl allows
> > userspace to get the minimum and maximum font size values.
> > 
> > Acked-by: Helge Deller <deller@gmx.de>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ...
> > --- a/drivers/tty/vt/vt_ioctl.c
> > +++ b/drivers/tty/vt/vt_ioctl.c
> > @@ -479,6 +479,17 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
> >   		break;
> >   	}
> >   
> > +	case KDFONTINFO: {
> > +		struct console_font_info fnt_info;
> > +
> > +		ret = con_font_info(vc, &fnt_info);
> > +		if (ret)
> > +			return ret;
> > +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
> 
> sizeof, I already commented.

I'm not sure I understand. sizeof(*up), but 'up' is 'void __user *up'.

> Now you leak info to userspace unless everyone sets everything in 
> fnt_info. IOW, do memset() above.

Yes. I miss it. Sorry.

-- 
Rgrds, legion