drivers/tty/vt/vc_screen.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
From: Thomas Weißschuh <linux@weissschuh.net>
Commit 226fae124b2d
("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
moved the call to vcs_vc() into the loop.
While doing this it also moved the unconditional assignment of
"ret = -ENXIO".
This unconditional assignment was valid outside the loop but within it
it clobbers the actual value of ret.
To avoid this only assign "ret = -ENXIO" when actually needed.
Reported-by: Storm Dragon <stormdragon2976@gmail.com>
Link: https://lore.kernel.org/lkml/Y%2FKS6vdql2pIsCiI@hotmail.com/
Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
@Storm Could you validate this patch?
---
drivers/tty/vt/vc_screen.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index f566eb1839dc..2ef519a40a87 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
unsigned int this_round, skip = 0;
int size;
- ret = -ENXIO;
vc = vcs_vc(inode, &viewed);
- if (!vc)
+ if (!vc) {
+ ret = -ENXIO;
goto unlock_out;
+ }
/* Check whether we are above size each round,
* as copy_to_user at the end of this loop
base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
--
2.39.2
On Mon, Feb 20, 2023 at 06:46:12AM +0000, linux@weissschuh.net wrote: >From: Thomas Weißschuh <linux@weissschuh.net> > >@Storm Could you validate this patch? I am willing to test, but I have almost no experience doing anything with the kernel other than upgrading it from time to time. Can you send me some instructions for testing this? Thanks, Storm
On Mon, Feb 20, 2023 at 11:06:08AM -0500, Storm Dragon wrote: > On Mon, Feb 20, 2023 at 06:46:12AM +0000, linux@weissschuh.net wrote: > > From: Thomas Weißschuh <linux@weissschuh.net> > > > > @Storm Could you validate this patch? > > > I am willing to test, but I have almost no experience doing anything > with the kernel other than upgrading it from time to time. Can you send > me some instructions for testing this? Sorry for the long delay, but this should be fixed in the current round of stable kernels. Can you try the following: pacman -U https://mirrors.edge.kernel.org/archlinux/testing/os/x86_64/linux-6.2.2.arch1-1-x86_64.pkg.tar.zst Thomas
On Fri, Mar 03, 2023 at 09:12:50PM +0000, Thomas Weißschuh wrote: >Sorry for the long delay, but this should be fixed in the current round >of stable kernels. Can you try the following: > >pacman -U https://mirrors.edge.kernel.org/archlinux/testing/os/x86_64/linux-6.2.2.arch1-1-x86_64.pkg.tar.zst > >Thomas I have installed the package above. My screen reader is behaving much better now. Interestingly, however, trying to cat the /dev/vcs device still shows the following: cat: /dev/vcs: No such device or address cat: /dev/vcsa: No such device or address cat: /dev/vcsa1: No such device or address Is this expected behavior?
On Fri, Mar 03, 2023 at 04:46:21PM -0500, Storm Dragon wrote: > On Fri, Mar 03, 2023 at 09:12:50PM +0000, Thomas Weißschuh wrote: > > Sorry for the long delay, but this should be fixed in the current round > > of stable kernels. Can you try the following: > > > > pacman -U https://mirrors.edge.kernel.org/archlinux/testing/os/x86_64/linux-6.2.2.arch1-1-x86_64.pkg.tar.zst > > > > Thomas > > I have installed the package above. My screen reader is behaving much > better now. Interestingly, however, trying to cat the /dev/vcs device > still shows the following: > > cat: /dev/vcs: No such device or address > > cat: /dev/vcsa: No such device or address > > cat: /dev/vcsa1: No such device or address > > Is this expected behavior? No it isn't. Is this reliably reproducible? I doesn't happen on my side. Maybe you can provide more detailed reproduction steps. Just to be sure; did you reboot into the new kernel? Does this mean the screenreader now works correctly or is it still broken somehow? Thomas
On Fri, Mar 03, 2023 at 11:25:00PM +0000, Thomas Weißschuh wrote: >Does this mean the screenreader now works correctly or is it still >broken somehow? > >Thomas I have still been testing this kernel. Most things work as expected, but the pasting functionality for Fenrir's clipboard is broken. After checking into the problem, it seems that tiocsti is disabled, and that is causing the problem. Was that something done in this test kernel only, or will that be the default for all new Arch kernels? If it is the default, is there a way to turn it back on? I tried the following: [storm@mjollnir ~] $ sudo sysctl dev.tty.legacy_tiocsti=1 sysctl: setting key "dev.tty.legacy_tiocsti": Invalid argument Thanks, Storm
On Sat, Mar 04, 2023 at 02:58:46PM -0500, Storm Dragon wrote: > On Fri, Mar 03, 2023 at 11:25:00PM +0000, Thomas Weißschuh wrote: > > > Does this mean the screenreader now works correctly or is it still > > broken somehow? > > > > Thomas > > I have still been testing this kernel. Most things work as expected, but > the pasting functionality for Fenrir's clipboard is broken. After > checking into the problem, it seems that tiocsti is disabled, and that > is causing the problem. Was that something done in this test kernel > only, or will that be the default for all new Arch kernels? If it is the > default, is there a way to turn it back on? I tried the following: > > [storm@mjollnir ~] $ sudo sysctl dev.tty.legacy_tiocsti=1 > sysctl: setting key "dev.tty.legacy_tiocsti": Invalid argument The sysctl to enable the ioctl should be fixed in 6.2.3: pacman -U https://mirrors.edge.kernel.org/archlinux/testing/os/x86_64/linux-6.2.3.arch1-1-x86_64.pkg.tar.zst Thomas
On Sat, Mar 04, 2023 at 02:58:46PM -0500, Storm Dragon wrote:
> On Fri, Mar 03, 2023 at 11:25:00PM +0000, Thomas Weißschuh wrote:
>
> > Does this mean the screenreader now works correctly or is it still
> > broken somehow?
> >
> > Thomas
>
> I have still been testing this kernel. Most things work as expected, but
> the pasting functionality for Fenrir's clipboard is broken. After
> checking into the problem, it seems that tiocsti is disabled, and that
> is causing the problem. Was that something done in this test kernel
> only, or will that be the default for all new Arch kernels? If it is the
> default, is there a way to turn it back on?
By now this package has been promoted to [core] so it is in fact the
default ArchLinux kernel.
As a workaround you can use the "linux-lts" package that now also
carries the fix for your original problem but not the code responsible
for the new issue.
pacman -Sy linux-lts
> I tried the following:
>
> [storm@mjollnir ~] $ sudo sysctl dev.tty.legacy_tiocsti=1
> sysctl: setting key "dev.tty.legacy_tiocsti": Invalid argument
This is indeed the correct way to enable the feature again.
It seems that the commit that introduced this sysctl[0] depends on
another commit [1] to be applied. But the 6.2.2 stable kernel is missing
the requirement.
I'll validate that this indeed is the issue and will then send a formal
request for backporting.
[0] 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled")
[1] f1aa2eb5ea05 ("sysctl: fix proc_dobool() usability")
On Fri, Mar 03, 2023 at 11:25:00PM +0000, Thomas Weißschuh wrote: >Does this mean the screenreader now works correctly or is it still >broken somehow? > >Thomas I forgot to answer the screen reader question. I hadn't rebooted in a while, and I forgot that the work around Chrys put in place for Fenrir does really well until the screen fills up. So, right after I sent the message saying it was working better, it reverted to doing its weird jumps and things caused by the bug. Now that the correct version is actually running, it really is better. Thanks, Storm
On Fri, Mar 03, 2023 at 11:25:00PM +0000, Thomas Weißschuh wrote: >No it isn't. > >Is this reliably reproducible? I doesn't happen on my side. >Maybe you can provide more detailed reproduction steps. > >Just to be sure; did you reboot into the new kernel? > >Does this mean the screenreader now works correctly or is it still >broken somehow? > >Thomas I found the problem when I was writing the steps I did to reproduce it. I did install the kernel, I did reboot, but I did not uninstall the lts kernel I was using currently, nor did I run grub-mkconfig. Now that I have done things the right way, it works as expected. Everything is awesome again. Thanks, and sorry for the false alarm. Storm
On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
> From: Thomas Weißschuh <linux@weissschuh.net>
>
> Commit 226fae124b2d
> ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> moved the call to vcs_vc() into the loop.
> While doing this it also moved the unconditional assignment of
> "ret = -ENXIO".
> This unconditional assignment was valid outside the loop but within it
> it clobbers the actual value of ret.
>
> To avoid this only assign "ret = -ENXIO" when actually needed.
Not sure -- I cannot find it -- but hasn't George fixed this yet?
> Reported-by: Storm Dragon <stormdragon2976@gmail.com>
> Link: https://lore.kernel.org/lkml/Y%2FKS6vdql2pIsCiI@hotmail.com/
> Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>
> ---
>
> @Storm Could you validate this patch?
> ---
> drivers/tty/vt/vc_screen.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
> index f566eb1839dc..2ef519a40a87 100644
> --- a/drivers/tty/vt/vc_screen.c
> +++ b/drivers/tty/vt/vc_screen.c
> @@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> unsigned int this_round, skip = 0;
> int size;
>
> - ret = -ENXIO;
> vc = vcs_vc(inode, &viewed);
> - if (!vc)
> + if (!vc) {
> + ret = -ENXIO;
> goto unlock_out;
> + }
>
> /* Check whether we are above size each round,
> * as copy_to_user at the end of this loop
>
> base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
--
js
suse labs
+Cc people who were involved in the original thread.
On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
> On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
> > From: Thomas Weißschuh <linux@weissschuh.net>
> >
> > Commit 226fae124b2d
> > ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> > moved the call to vcs_vc() into the loop.
> > While doing this it also moved the unconditional assignment of
> > "ret = -ENXIO".
> > This unconditional assignment was valid outside the loop but within it
> > it clobbers the actual value of ret.
> >
> > To avoid this only assign "ret = -ENXIO" when actually needed.
>
> Not sure -- I cannot find it -- but hasn't George fixed this yet?
Indeed there was a proposed fix at
https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
Linus had some suggestions so it was not applied as is.
I'm not sure what the current state is.
George, do you have something in the pipeline?
I also tested the patch proposed by Linus as attachment and that works.
(The small inline patch snippet doesn't)
> > Reported-by: Storm Dragon <stormdragon2976@gmail.com>
> > Link: https://lore.kernel.org/lkml/Y%2FKS6vdql2pIsCiI@hotmail.com/
> > Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> >
> > ---
> >
> > @Storm Could you validate this patch?
> > ---
> > drivers/tty/vt/vc_screen.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
> > index f566eb1839dc..2ef519a40a87 100644
> > --- a/drivers/tty/vt/vc_screen.c
> > +++ b/drivers/tty/vt/vc_screen.c
> > @@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> > unsigned int this_round, skip = 0;
> > int size;
> > - ret = -ENXIO;
> > vc = vcs_vc(inode, &viewed);
> > - if (!vc)
> > + if (!vc) {
> > + ret = -ENXIO;
> > goto unlock_out;
> > + }
> > /* Check whether we are above size each round,
> > * as copy_to_user at the end of this loop
> >
> > base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
>
> --
> js
> suse labs
>
On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
> +Cc people who were involved in the original thread.
>
> On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
>> On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
>>> From: Thomas Weißschuh <linux@weissschuh.net>
>>>
>>> Commit 226fae124b2d
>>> ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
>>> moved the call to vcs_vc() into the loop.
>>> While doing this it also moved the unconditional assignment of
>>> "ret = -ENXIO".
>>> This unconditional assignment was valid outside the loop but within it
>>> it clobbers the actual value of ret.
>>>
>>> To avoid this only assign "ret = -ENXIO" when actually needed.
>> Not sure -- I cannot find it -- but hasn't George fixed this yet?
> Indeed there was a proposed fix at
> https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
>
> Linus had some suggestions so it was not applied as is.
>
> I'm not sure what the current state is.
> George, do you have something in the pipeline?
Yes, that is in the pipeline:
https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
Linus suggested the fix, which was tested and submitted.
Jiri commented on the patch, which I believe was directed at Linus as he
suggested the fix.
George
>
> I also tested the patch proposed by Linus as attachment and that works.
> (The small inline patch snippet doesn't)
>
>>> Reported-by: Storm Dragon <stormdragon2976@gmail.com>
>>> Link: https://lore.kernel.org/lkml/Y%2FKS6vdql2pIsCiI@hotmail.com/
>>> Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
>>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>>>
>>> ---
>>>
>>> @Storm Could you validate this patch?
>>> ---
>>> drivers/tty/vt/vc_screen.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
>>> index f566eb1839dc..2ef519a40a87 100644
>>> --- a/drivers/tty/vt/vc_screen.c
>>> +++ b/drivers/tty/vt/vc_screen.c
>>> @@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>>> unsigned int this_round, skip = 0;
>>> int size;
>>> - ret = -ENXIO;
>>> vc = vcs_vc(inode, &viewed);
>>> - if (!vc)
>>> + if (!vc) {
>>> + ret = -ENXIO;
>>> goto unlock_out;
>>> + }
>>> /* Check whether we are above size each round,
>>> * as copy_to_user at the end of this loop
>>>
>>> base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
>> --
>> js
>> suse labs
>>
On Tue, Feb 21, 2023 at 08:30:11AM -0500, George Kennedy wrote:
>
>
> On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
> > +Cc people who were involved in the original thread.
> >
> > On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
> > > On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
> > > > From: Thomas Weißschuh <linux@weissschuh.net>
> > > >
> > > > Commit 226fae124b2d
> > > > ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> > > > moved the call to vcs_vc() into the loop.
> > > > While doing this it also moved the unconditional assignment of
> > > > "ret = -ENXIO".
> > > > This unconditional assignment was valid outside the loop but within it
> > > > it clobbers the actual value of ret.
> > > >
> > > > To avoid this only assign "ret = -ENXIO" when actually needed.
> > > Not sure -- I cannot find it -- but hasn't George fixed this yet?
> > Indeed there was a proposed fix at
> > https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
> >
> > Linus had some suggestions so it was not applied as is.
> >
> > I'm not sure what the current state is.
> > George, do you have something in the pipeline?
>
> Yes, that is in the pipeline:
> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>
> Linus suggested the fix, which was tested and submitted.
>
> Jiri commented on the patch, which I believe was directed at Linus as he
> suggested the fix.
Thanks for the pointer!
I searched for it by its Fixes: tag.
The v2 has a different one than the v1.
To me the v1 Fixes: seems more correct, was the change
intentional?
> George
> >
> > I also tested the patch proposed by Linus as attachment and that works.
> > (The small inline patch snippet doesn't)
> >
> > > > Reported-by: Storm Dragon <stormdragon2976@gmail.com>
> > > > Link: https://lore.kernel.org/lkml/Y%2FKS6vdql2pIsCiI@hotmail.com/
> > > > Fixes: 226fae124b2d ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > >
> > > > ---
> > > >
> > > > @Storm Could you validate this patch?
> > > > ---
> > > > drivers/tty/vt/vc_screen.c | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
> > > > index f566eb1839dc..2ef519a40a87 100644
> > > > --- a/drivers/tty/vt/vc_screen.c
> > > > +++ b/drivers/tty/vt/vc_screen.c
> > > > @@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> > > > unsigned int this_round, skip = 0;
> > > > int size;
> > > > - ret = -ENXIO;
> > > > vc = vcs_vc(inode, &viewed);
> > > > - if (!vc)
> > > > + if (!vc) {
> > > > + ret = -ENXIO;
> > > > goto unlock_out;
> > > > + }
> > > > /* Check whether we are above size each round,
> > > > * as copy_to_user at the end of this loop
> > > >
> > > > base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
> > > --
> > > js
> > > suse labs
> > >
>
On Tue, Feb 21, 2023 at 08:30:11AM -0500, George Kennedy wrote:
>
>
> On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
> > +Cc people who were involved in the original thread.
> >
> > On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
> > > On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
> > > > From: Thomas Weißschuh <linux@weissschuh.net>
> > > >
> > > > Commit 226fae124b2d
> > > > ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
> > > > moved the call to vcs_vc() into the loop.
> > > > While doing this it also moved the unconditional assignment of
> > > > "ret = -ENXIO".
> > > > This unconditional assignment was valid outside the loop but within it
> > > > it clobbers the actual value of ret.
> > > >
> > > > To avoid this only assign "ret = -ENXIO" when actually needed.
> > > Not sure -- I cannot find it -- but hasn't George fixed this yet?
> > Indeed there was a proposed fix at
> > https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
> >
> > Linus had some suggestions so it was not applied as is.
> >
> > I'm not sure what the current state is.
> > George, do you have something in the pipeline?
>
> Yes, that is in the pipeline:
> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>
> Linus suggested the fix, which was tested and submitted.
>
> Jiri commented on the patch, which I believe was directed at Linus as he
> suggested the fix.
And I was waiting for a new version from you based on those comments :(
Can you fix that up and send?
thanks,
greg k-h
Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.
George, is there anything we can do to help you moving forward to
finally get this regression fixed? It seems (or am I missing something?)
everyone is waiting for you (see below) to act on the feedback Jiri
provided here:
https://lore.kernel.org/lkml/8dffe187-240d-746e-ed84-885ffd2785f6@kernel.org/
Side note: would be good to add a "Link:" tag pointing to the start of
this thread as well, but that's just a detail.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
On 21.02.23 14:50, Greg Kroah-Hartman wrote:
> On Tue, Feb 21, 2023 at 08:30:11AM -0500, George Kennedy wrote:
>> On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
>>> +Cc people who were involved in the original thread.
>>>
>>> On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
>>>> On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
>>>>> From: Thomas Weißschuh <linux@weissschuh.net>
>>>>>
>>>>> Commit 226fae124b2d
>>>>> ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
>>>>> moved the call to vcs_vc() into the loop.
>>>>> While doing this it also moved the unconditional assignment of
>>>>> "ret = -ENXIO".
>>>>> This unconditional assignment was valid outside the loop but within it
>>>>> it clobbers the actual value of ret.
>>>>>
>>>>> To avoid this only assign "ret = -ENXIO" when actually needed.
>>>> Not sure -- I cannot find it -- but hasn't George fixed this yet?
>>> Indeed there was a proposed fix at
>>> https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
>>>
>>> Linus had some suggestions so it was not applied as is.
>>>
>>> I'm not sure what the current state is.
>>> George, do you have something in the pipeline?
>>
>> Yes, that is in the pipeline:
>> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>>
>> Linus suggested the fix, which was tested and submitted.
>>
>> Jiri commented on the patch, which I believe was directed at Linus as he
>> suggested the fix.
>
> And I was waiting for a new version from you based on those comments :(
>
> Can you fix that up and send?
>
> thanks,
>
> greg k-h
#regzbot monitor:
https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
#regzbot poke
Hello Thomas,
On 2/27/2023 9:20 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
>
> George, is there anything we can do to help you moving forward to
> finally get this regression fixed? It seems (or am I missing something?)
> everyone is waiting for you (see below) to act on the feedback Jiri
> provided here:
>
> https://lore.kernel.org/lkml/8dffe187-240d-746e-ed84-885ffd2785f6@kernel.org/
>
> Side note: would be good to add a "Link:" tag pointing to the start of
> this thread as well, but that's just a detail.
I just sent the requested patch up for review.
https://lore.kernel.org/lkml/1677527001-17459-1-git-send-email-george.kennedy@oracle.com/
Last post on the previous patch that led to the requested patch:
https://lore.kernel.org/lkml/9e297f30-dc8c-ecac-f7a6-348ddbd4b928@leemhuis.info/
Thank you,
George
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> On 21.02.23 14:50, Greg Kroah-Hartman wrote:
>> On Tue, Feb 21, 2023 at 08:30:11AM -0500, George Kennedy wrote:
>>> On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
>>>> +Cc people who were involved in the original thread.
>>>>
>>>> On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
>>>>> On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
>>>>>> From: Thomas Weißschuh <linux@weissschuh.net>
>>>>>>
>>>>>> Commit 226fae124b2d
>>>>>> ("vc_screen: move load of struct vc_data pointer in vcs_read() to avoid UAF")
>>>>>> moved the call to vcs_vc() into the loop.
>>>>>> While doing this it also moved the unconditional assignment of
>>>>>> "ret = -ENXIO".
>>>>>> This unconditional assignment was valid outside the loop but within it
>>>>>> it clobbers the actual value of ret.
>>>>>>
>>>>>> To avoid this only assign "ret = -ENXIO" when actually needed.
>>>>> Not sure -- I cannot find it -- but hasn't George fixed this yet?
>>>> Indeed there was a proposed fix at
>>>> https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
>>>>
>>>> Linus had some suggestions so it was not applied as is.
>>>>
>>>> I'm not sure what the current state is.
>>>> George, do you have something in the pipeline?
>>> Yes, that is in the pipeline:
>>> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>>>
>>> Linus suggested the fix, which was tested and submitted.
>>>
>>> Jiri commented on the patch, which I believe was directed at Linus as he
>>> suggested the fix.
>> And I was waiting for a new version from you based on those comments :(
>>
>> Can you fix that up and send?
>>
>> thanks,
>>
>> greg k-h
> #regzbot monitor:
> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
> #regzbot poke
On 27.02.23 20:59, George Kennedy wrote:
> Hello Thomas,
>
> On 2/27/2023 9:20 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
>> Hi, this is your Linux kernel regression tracker. Top-posting for once,
>> to make this easily accessible to everyone.
>>
>> George, is there anything we can do to help you moving forward to
>> finally get this regression fixed? It seems (or am I missing something?)
>> everyone is waiting for you (see below) to act on the feedback Jiri
>> provided here:
>>
>> https://lore.kernel.org/lkml/8dffe187-240d-746e-ed84-885ffd2785f6@kernel.org/
>>
>> Side note: would be good to add a "Link:" tag pointing to the start of
>> this thread as well, but that's just a detail.
>
> I just sent the requested patch up for review.
>
> https://lore.kernel.org/lkml/1677527001-17459-1-git-send-email-george.kennedy@oracle.com/
Thx for handling this!
And I see it very quickly made its way to mainline. :-D
Ciao, Thorsten
>> On 21.02.23 14:50, Greg Kroah-Hartman wrote:
>>> On Tue, Feb 21, 2023 at 08:30:11AM -0500, George Kennedy wrote:
>>>> On 2/20/2023 11:34 AM, Thomas Weißschuh wrote:
>>>>> +Cc people who were involved in the original thread.
>>>>>
>>>>> On Mon, Feb 20, 2023 at 12:48:59PM +0100, Jiri Slaby wrote:
>>>>>> On 20. 02. 23, 7:46, linux@weissschuh.net wrote:
>>>>>>> From: Thomas Weißschuh <linux@weissschuh.net>
>>>>>>>
>>>>>>> Commit 226fae124b2d
>>>>>>> ("vc_screen: move load of struct vc_data pointer in vcs_read() to
>>>>>>> avoid UAF")
>>>>>>> moved the call to vcs_vc() into the loop.
>>>>>>> While doing this it also moved the unconditional assignment of
>>>>>>> "ret = -ENXIO".
>>>>>>> This unconditional assignment was valid outside the loop but
>>>>>>> within it
>>>>>>> it clobbers the actual value of ret.
>>>>>>>
>>>>>>> To avoid this only assign "ret = -ENXIO" when actually needed.
>>>>>> Not sure -- I cannot find it -- but hasn't George fixed this yet?
>>>>> Indeed there was a proposed fix at
>>>>> https://lore.kernel.org/lkml/1675704844-17228-1-git-send-email-george.kennedy@oracle.com/
>>>>>
>>>>> Linus had some suggestions so it was not applied as is.
>>>>>
>>>>> I'm not sure what the current state is.
>>>>> George, do you have something in the pipeline?
>>>> Yes, that is in the pipeline:
>>>> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>>>>
>>>> Linus suggested the fix, which was tested and submitted.
>>>>
>>>> Jiri commented on the patch, which I believe was directed at Linus
>>>> as he
>>>> suggested the fix.
>>> And I was waiting for a new version from you based on those comments :(
>>>
>>> Can you fix that up and send?
>>>
>>> thanks,
>>>
>>> greg k-h
>> #regzbot monitor:
>> https://lore.kernel.org/lkml/1675774098-17722-1-git-send-email-george.kennedy@oracle.com/
>> #regzbot poke
>
>
>
© 2016 - 2025 Red Hat, Inc.