From: Or Cohen <orcohen@paloaltonetworks.com>
To: Jiri Slaby <jslaby@suse.com>
Cc: Nicolas Pitre <nico@fluxnic.net>,
Greg KH <gregkh@linuxfoundation.org>,
textshell@uchuujin.de, Daniel Vetter <daniel.vetter@ffwll.ch>,
sam@ravnborg.org, mpatocka@redhat.com, ghalat@redhat.com,
linux-kernel@vger.kernel.org, jwilk@jwilk.net,
Nadav Markus <nmarkus@paloaltonetworks.com>,
syzkaller@googlegroups.com
Subject: Re: Bug report - slab-out-of-bounds in vcs_scr_readw
Date: Tue, 5 Nov 2019 01:03:23 -0800 [thread overview]
Message-ID: <CAM6JnLe88xf8hO0F=_Ni+irNt40+987tHmz9ZjppgxhnMnLxpw@mail.gmail.com> (raw)
In-Reply-To: <c30fc539-68a8-65d7-226c-6f8e6fd8bdfb@suse.com>
@Nicolas Pitre - I agree with you, "vcs_size" may return a negative
error code, so the patch is correct but as @jslaby@suse.com said it
won't fix the issue.
In my debugging session, "vcs_size" returns a positive integer ( 8000
decimal ) and the bug still triggers.
Maybe it's related to the following logic in "vcs_size"? ( not sure
about that.. )
221 if (use_attributes(inode)) {
222 if (use_unicode(inode))
223 return -EOPNOTSUPP;
224 size = 2*size + HEADER_SIZE;
225 } else if (use_unicode(inode))
226 size *= 4;
227 return size;
Why in the case of "use_unicode(inode)" size is multiplied by 4 and
not 2? ( as we can see in line 224 )
On Mon, Nov 4, 2019 at 10:54 PM Jiri Slaby <jslaby@suse.com> wrote:
>
> On 04. 11. 19, 19:33, Nicolas Pitre wrote:
> > On Mon, 4 Nov 2019, Or Cohen wrote:
> >
> >> @gregkh@linuxfoundation.org @nico@fluxnic.net - Thanks for the quick response.
> >> @gregkh@linuxfoundation.org - Regarding your question, I don't think
> >> the 1 byte buffer is related to the problem. ( it's just was there in
> >> the initial reproducer the fuzzer created, and I forgot to remove it
> >> while reducing code from the reproducer ).
> >
> > I think I know what the problem is. I have no time to test it though.
> >
> > Please try this (untested) patch. Also please try running the same test
> > code but with vcsa6 in addition to vcsu6 to be sure.
> >
> > ---------- >8
> > Subject: [PATCH] vcs: add missing validation on vcs_size() returned value
> >
> > One usage instance didn't account for the fact that vcs_size() may
> > return a negative error code.
> >
> > Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> >
> > diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
> > index 1f042346e7..fa07d79027 100644
> > --- a/drivers/tty/vt/vc_screen.c
> > +++ b/drivers/tty/vt/vc_screen.c
> > @@ -474,6 +474,10 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> > goto unlock_out;
> >
> > size = vcs_size(inode);
> > + if (size < 0) {
> > + ret = size;
> > + goto unlock_out;
> > + }
> > ret = -EINVAL;
> > if (pos < 0 || pos > size)
> > goto unlock_out;
>
> pos must be >= 0, so "pos > size" would catch this case as a side
> effect, or am I missing something? That being said, the patch is
> correct, but won't fix the issue IMO.
>
> thanks,
> --
> js
> suse labs
next prev parent reply other threads:[~2019-11-05 10:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAM6JnLeEnvjjQPyLeh+8dt5wGNud_vks5k_eXJZy2T1H7ao=hQ@mail.gmail.com>
2019-11-04 15:24 ` Bug report - slab-out-of-bounds in vcs_scr_readw Greg KH
2019-11-04 15:50 ` Nicolas Pitre
2019-11-04 16:15 ` Or Cohen
2019-11-04 18:33 ` Nicolas Pitre
2019-11-05 6:54 ` Jiri Slaby
2019-11-05 9:03 ` Or Cohen [this message]
2019-11-05 9:14 ` Jiri Slaby
2019-11-05 9:33 ` Nicolas Pitre
2019-11-05 10:29 ` Jiri Slaby
2019-11-05 10:53 ` Dmitry Vyukov
2019-11-05 13:44 ` Nicolas Pitre
2019-11-06 17:01 ` Or Cohen
2019-11-26 21:55 ` Nicolas Pitre
2019-11-27 6:45 ` Greg KH
2019-11-27 16:24 ` Nicolas Pitre
2019-11-27 16:30 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAM6JnLe88xf8hO0F=_Ni+irNt40+987tHmz9ZjppgxhnMnLxpw@mail.gmail.com' \
--to=orcohen@paloaltonetworks.com \
--cc=daniel.vetter@ffwll.ch \
--cc=ghalat@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=jwilk@jwilk.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=nico@fluxnic.net \
--cc=nmarkus@paloaltonetworks.com \
--cc=sam@ravnborg.org \
--cc=syzkaller@googlegroups.com \
--cc=textshell@uchuujin.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.