Re: Bug: broken /proc/kcore in 6.13

[Alexandre Ferrieux <alexandre.ferrieux@gmail.com>]

On 17/01/2025 15:44, Lorenzo Stoakes wrote:
> 
>> Alexandre Ferrieux <alexandre.ferrieux@gmail.com> wrote:
>>
>> > Hi,
>> >
>> > Somewhere in the 6.13 branch (not bisected yet, sorry), it stopped being
>> > possible to disassemble the running kernel from gdb through /proc/kcore.
> 
> Thanks for the report! Much appreciated.
> 
> I may try to bisect here also unless you're close to finding the commit that
> broke this?

I'm currently homing in on copy_page_to_iter_nofault(), will report shortly :)

> Yikes, this is my fault. Sorry about that!

Wow. I'm so happy we connected, no problem :)

> There was some discussion at the time about the infinite loop, obviously with
> the understanding that vread_iter() should never return 0 in this scenario
> (where we had identified the _category_ of kernel memory being accessed), which
> is obviously now rendered false.
> 
> The fact that it can is (obviously) rather problematic... obviously we need to
> patch this, if this were possible in real scenarios in the past we would
> probably also want to backport a fix.
> 
> In any case I think we need an explicit check here no matter the cause so we can
> never loop like this. This was just an oversight at the time given this is a
> documented behaviour.
> 
> My instinct is to error out if this returns 0, because that would indicate that
> the address is not part of the vmalloc area.

Yes, I did the naive patch below; it does the job, breaking out of the loop, but
does not cure the access problem, so gdb just sees zeroes :(

diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index e376f48c4b8b..8c5f29240542 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -531,7 +531,13 @@ static ssize_t read_kcore_iter(struct kiocb *iocb, struct
iov_iter *iter)
                         * again until we are done.
                         */
                        while (true) {
-                               read += vread_iter(iter, src, left);
+                               long res;
+                               res = vread_iter(iter, src, left);
+                               if (!res) {
+                                       ret = -EFAULT;
+                                       goto out;
+                               }
+                               read += res;
                                if (read == tsz)
                                        break;

> But then it seems add_modules_range() is just adding the module range under
> category KCORE_VMALLOC despite it not being in the vmalloc range :/ which is
> really odd. This was added a long time ago so clearly not what triggered this
> but odd.
> 
> In any case, let me go have a look at this...

Ok, staying eagerly tuned !

Best regards,

-Alex