[PATCH 0/2] tty: vcc: Some small vcc bugfixes found by code scans

[PATCH 0/2] tty: vcc: Some small vcc bugfixes found by code scans

[Greg Kroah-Hartman]

Joshua at AISLE has been kind enough to run a bunch of scans on the
tty/serial code and here are two minor fixes for the vcc code that the
tools found.

One note, is this code even used anymore?  Seems to be sparc-specific,
just wanted to know if this platform is still alive?  If not, I'll be
glad to delete this file :)

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Joshua Rogers (2):
      tty: vcc: zero-initialize control packet in vcc_send_ctl()
      tty: vcc: hold port lock when clearing tty pointer in vcc_cleanup

 drivers/tty/vcc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
---
base-commit: 8ba098e6b6ff0db8edf28528d1552be261af30d4
change-id: 20260731-aisle-tty-vcc-8ea9ace26f6f

Best regards,
--  
Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 1/2] tty: vcc: zero-initialize control packet in vcc_send_ctl()

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The stack-allocated struct vio_vcc pkt was partially initialized,
leaving the tag.stype_env field uninitialized before being sent via
ldc_write(), potentially leaking kernel stack data to the LDC peer.

Assisted-by: AISLE:Snapshot
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/vcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
index 27a55465bf5e..3947bd2b75ac 100644
--- a/drivers/tty/vcc.c
+++ b/drivers/tty/vcc.c
@@ -492,7 +492,7 @@ static ssize_t domain_show(struct device *dev,
 
 static int vcc_send_ctl(struct vcc_port *port, int ctl)
 {
-	struct vio_vcc pkt;
+	struct vio_vcc pkt = {};
 	int rv;
 
 	pkt.tag.type = VIO_TYPE_CTRL;

-- 
2.55.0

Re: [PATCH 1/2] tty: vcc: zero-initialize control packet in vcc_send_ctl()

[Jiri Slaby]

On 31. 07. 26, 10:15, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> The stack-allocated struct vio_vcc pkt was partially initialized,
> leaving the tag.stype_env field uninitialized before being sent via
> ldc_write(), potentially leaking kernel stack data to the LDC peer.
> 
> Assisted-by: AISLE:Snapshot
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   drivers/tty/vcc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> index 27a55465bf5e..3947bd2b75ac 100644
> --- a/drivers/tty/vcc.c
> +++ b/drivers/tty/vcc.c
> @@ -492,7 +492,7 @@ static ssize_t domain_show(struct device *dev,
>   
>   static int vcc_send_ctl(struct vcc_port *port, int ctl)
>   {
> -	struct vio_vcc pkt;
> +	struct vio_vcc pkt = {};

Note this needs not initialize holes. I believe there are none here, but 
memset() is usually preferred/safer.

thanks,
-- 
js
suse labs

Re: [PATCH 1/2] tty: vcc: zero-initialize control packet in vcc_send_ctl()

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 10:29:14AM +0200, Jiri Slaby wrote:
> On 31. 07. 26, 10:15, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > The stack-allocated struct vio_vcc pkt was partially initialized,
> > leaving the tag.stype_env field uninitialized before being sent via
> > ldc_write(), potentially leaking kernel stack data to the LDC peer.
> > 
> > Assisted-by: AISLE:Snapshot
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   drivers/tty/vcc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> > index 27a55465bf5e..3947bd2b75ac 100644
> > --- a/drivers/tty/vcc.c
> > +++ b/drivers/tty/vcc.c
> > @@ -492,7 +492,7 @@ static ssize_t domain_show(struct device *dev,
> >   static int vcc_send_ctl(struct vcc_port *port, int ctl)
> >   {
> > -	struct vio_vcc pkt;
> > +	struct vio_vcc pkt = {};
> 
> Note this needs not initialize holes.

Does it?  I keep forgetting this if it's really true or not.  I think
we've been through this in the past but I can't find the archives.

> I believe there are none here, but memset() is usually
> preferred/safer.

Last I remember, a long time ago, the compiler would just use memset()
for this, and hit the holes.  Or is that a compiler-specific issue?

Yes, it's more obvious, and I'll be glad to change it, but figuring this
out for real would be great :)

thanks,

greg k-h

Re: [PATCH 1/2] tty: vcc: zero-initialize control packet in vcc_send_ctl()

[Jiri Slaby]

On 31. 07. 26, 10:32, Greg Kroah-Hartman wrote:
> On Fri, Jul 31, 2026 at 10:29:14AM +0200, Jiri Slaby wrote:
>> On 31. 07. 26, 10:15, Greg Kroah-Hartman wrote:
>>> From: Joshua Rogers <linux@joshua.hu>
>>>
>>> The stack-allocated struct vio_vcc pkt was partially initialized,
>>> leaving the tag.stype_env field uninitialized before being sent via
>>> ldc_write(), potentially leaking kernel stack data to the LDC peer.
>>>
>>> Assisted-by: AISLE:Snapshot
>>> Cc: stable <stable@kernel.org>
>>> Signed-off-by: Joshua Rogers <linux@joshua.hu>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>    drivers/tty/vcc.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
>>> index 27a55465bf5e..3947bd2b75ac 100644
>>> --- a/drivers/tty/vcc.c
>>> +++ b/drivers/tty/vcc.c
>>> @@ -492,7 +492,7 @@ static ssize_t domain_show(struct device *dev,
>>>    static int vcc_send_ctl(struct vcc_port *port, int ctl)
>>>    {
>>> -	struct vio_vcc pkt;
>>> +	struct vio_vcc pkt = {};
>>
>> Note this needs not initialize holes.
> 
> Does it?  I keep forgetting this if it's really true or not.  I think
> we've been through this in the past but I can't find the archives.
> 
>> I believe there are none here, but memset() is usually
>> preferred/safer.
> 
> Last I remember, a long time ago, the compiler would just use memset()
> for this, and hit the holes.  Or is that a compiler-specific issue?
> 
> Yes, it's more obvious, and I'll be glad to change it, but figuring this
> out for real would be great :)

OK, so the standard (C17) does not guarantee zeroing.

What I found though:

* both clang and gcc zero the full struct for "{}". There was a bug in 
gcc < 8 where it did not.

* this is not true for designated initializer, cf. ce50039be49e ("net: 
sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak")).


Adding Kees who is the author of:
====
Memory initialization
---------------------

Memory copied to userspace must always be fully initialized. If not
explicitly memset(), this will require changes to the compiler to make
sure structure holes are cleared.
====

He might aware of more bugs.



FWIW, {} here should be safe for two reasons:
* not a designated initializer
* no holes (IMO)

thanks,
-- 
js
suse labs

Re: [PATCH 1/2] tty: vcc: zero-initialize control packet in vcc_send_ctl()

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 11:16:57AM +0200, Jiri Slaby wrote:
> On 31. 07. 26, 10:32, Greg Kroah-Hartman wrote:
> > On Fri, Jul 31, 2026 at 10:29:14AM +0200, Jiri Slaby wrote:
> > > On 31. 07. 26, 10:15, Greg Kroah-Hartman wrote:
> > > > From: Joshua Rogers <linux@joshua.hu>
> > > > 
> > > > The stack-allocated struct vio_vcc pkt was partially initialized,
> > > > leaving the tag.stype_env field uninitialized before being sent via
> > > > ldc_write(), potentially leaking kernel stack data to the LDC peer.
> > > > 
> > > > Assisted-by: AISLE:Snapshot
> > > > Cc: stable <stable@kernel.org>
> > > > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > ---
> > > >    drivers/tty/vcc.c | 2 +-
> > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> > > > index 27a55465bf5e..3947bd2b75ac 100644
> > > > --- a/drivers/tty/vcc.c
> > > > +++ b/drivers/tty/vcc.c
> > > > @@ -492,7 +492,7 @@ static ssize_t domain_show(struct device *dev,
> > > >    static int vcc_send_ctl(struct vcc_port *port, int ctl)
> > > >    {
> > > > -	struct vio_vcc pkt;
> > > > +	struct vio_vcc pkt = {};
> > > 
> > > Note this needs not initialize holes.
> > 
> > Does it?  I keep forgetting this if it's really true or not.  I think
> > we've been through this in the past but I can't find the archives.
> > 
> > > I believe there are none here, but memset() is usually
> > > preferred/safer.
> > 
> > Last I remember, a long time ago, the compiler would just use memset()
> > for this, and hit the holes.  Or is that a compiler-specific issue?
> > 
> > Yes, it's more obvious, and I'll be glad to change it, but figuring this
> > out for real would be great :)
> 
> OK, so the standard (C17) does not guarantee zeroing.
> 
> What I found though:
> 
> * both clang and gcc zero the full struct for "{}". There was a bug in gcc <
> 8 where it did not.
> 
> * this is not true for designated initializer, cf. ce50039be49e ("net:
> sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak")).

Ok, as our compiler is newer than these versions, and this isn't a
designated initializer, we should be ok.

> 
> Adding Kees who is the author of:
> ====
> Memory initialization
> ---------------------
> 
> Memory copied to userspace must always be fully initialized. If not
> explicitly memset(), this will require changes to the compiler to make
> sure structure holes are cleared.
> ====
> 
> He might aware of more bugs.
> 
> 
> 
> FWIW, {} here should be safe for two reasons:
> * not a designated initializer
> * no holes (IMO)

Great, I'll keep this as-is.  Now if only anyone can tell me if this
code is even used anymore :)

thanks,

greg k-h

[PATCH 2/2] tty: vcc: hold port lock when clearing tty pointer in vcc_cleanup

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

vcc_cleanup() sets port->tty to NULL without holding port->lock, racing
with vcc_event() LDC callbacks that read port->tty under port->lock and
then use tty->port. This can cause a use-after-free when the callback
dereferences tty->port after cleanup has already destroyed and freed it.

Assisted-by: AISLE:Snapshot
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/vcc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
index 3947bd2b75ac..03ed8c692acd 100644
--- a/drivers/tty/vcc.c
+++ b/drivers/tty/vcc.c
@@ -983,10 +983,13 @@ static int vcc_install(struct tty_driver *driver, struct tty_struct *tty)
 static void vcc_cleanup(struct tty_struct *tty)
 {
 	struct vcc_port *port;
+	unsigned long flags;
 
 	port = vcc_get(tty->index, true);
 	if (port) {
+		spin_lock_irqsave(&port->lock, flags);
 		port->tty = NULL;
+		spin_unlock_irqrestore(&port->lock, flags);
 
 		if (port->removed) {
 			vcc_table_remove(tty->index);

-- 
2.55.0

Re: [PATCH 2/2] tty: vcc: hold port lock when clearing tty pointer in vcc_cleanup

[Jiri Slaby]

On 31. 07. 26, 10:15, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> vcc_cleanup() sets port->tty to NULL without holding port->lock, racing
> with vcc_event() LDC callbacks that read port->tty under port->lock and
> then use tty->port. This can cause a use-after-free when the callback
> dereferences tty->port after cleanup has already destroyed and freed it.
> 
> Assisted-by: AISLE:Snapshot
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   drivers/tty/vcc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> index 3947bd2b75ac..03ed8c692acd 100644
> --- a/drivers/tty/vcc.c
> +++ b/drivers/tty/vcc.c
> @@ -983,10 +983,13 @@ static int vcc_install(struct tty_driver *driver, struct tty_struct *tty)
>   static void vcc_cleanup(struct tty_struct *tty)
>   {
>   	struct vcc_port *port;
> +	unsigned long flags;
>   
>   	port = vcc_get(tty->index, true);
>   	if (port) {
> +		spin_lock_irqsave(&port->lock, flags);
>   		port->tty = NULL;
> +		spin_unlock_irqrestore(&port->lock, flags);

For these, I like

scope_guard(spinlock_irqsave, &port->lock)
   port->tty = NULL;

more, as well.

thanks,
-- 
js
suse labs