linux-kernel - Re: [PATCH] tty: nozomi: Return an error when failing to create the sysfs

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: <CAMhUBj=nTiTu8wwjk0=WGE=nJPWtb+kSq_u=MW_FF1DHLE-55A@mail.gmail.com>
Date:   Thu, 9 Jun 2022 19:03:32 +0800
From:   Zheyu Ma <zheyuma97@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Jiri Slaby <jirislaby@...nel.org>, fseidel@...e.de,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tty: nozomi: Return an error when failing to create the sysfs

On Thu, Jun 9, 2022 at 4:38 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Thu, Jun 09, 2022 at 04:31:33PM +0800, Zheyu Ma wrote:
> > The driver does not handle the error of the creation of sysfs, resulting
> > in duplicate file names being created.
> >
> > The following log can reveal it:
> >
> > [   52.907211] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:05.0/card_type'
>
> How is the same file being created in a normal codepath?
>
> Is the same device being registered twice somehow?

In fact, I tried to load the nozomi driver twice.
In the first load, the driver failed at tty_port_register_device(),
performed error handling and returned an error, but by this time the
make_sysfs_files() had been executed and the sysfs had been created.
In the second load, the make_sysfs_files() is executed again and this
warning is returned.

> > [   52.907224] Call Trace:
> > [   52.907269]  sysfs_add_file_mode_ns+0x23f/0x2b0
> > [   52.907281]  sysfs_create_file_ns+0xe9/0x170
> > [   52.907321]  nozomi_card_init+0x97f/0x12c0 [nozomi]
> >
> > Fix this bug by returning an error when failing to create the sysfs.
> >
> > Fixes: 20fd1e3bea55 ("nozomi driver")
> > Signed-off-by: Zheyu Ma <zheyuma97@...il.com>
> > ---
> >  drivers/tty/nozomi.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
> > index 0454c78deee6..d0ad1b9898f5 100644
> > --- a/drivers/tty/nozomi.c
> > +++ b/drivers/tty/nozomi.c
> > @@ -1282,14 +1282,26 @@ static ssize_t open_ttys_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RO(open_ttys);
> >
> > -static void make_sysfs_files(struct nozomi *dc)
> > +static int make_sysfs_files(struct nozomi *dc)
> >  {
> > -     if (device_create_file(&dc->pdev->dev, &dev_attr_card_type))
> > +     int err;
> > +
> > +     err = device_create_file(&dc->pdev->dev, &dev_attr_card_type);
>
> Drivers shouldn't be calling this function anyway, it's a race
> condition.  How about switching this to use the proper device groups
> functionality instead which moves all of the handling of the sysfs files
> to the driver core so that the driver can not get it wrong?

Thanks for your reminder, since I'm not familiar with this function, I
will learn how to use the device groups functionality and revise the
patch.

> That should solve the error handling case.  The fact that this is trying
> to be created twice is a symptom of a worse problem here.  How are you
> duplicating this?

As mentioned above, I tried to load the nozomi driver twice and
make_sysfs_files() is executed twice.

thanks,

Zheyu Ma