ChromeOS public bug: https://issuetracker.google.com/issues/268356054

What's the point of setting up a system such that a debugger can't write to  inferior memory using /proc/pid/mem, but it can write via ptrace?  That gives you no extra protection.  I think the right fix is to just make it so that the kernel allows writes if the calling process has ptrace permissions over the target pid, even if it prohibits other processes from writing.

The workaround on the gdb side was for very old kernels, and for example will not work with non-stop mode, and of course does not support writing memory while threads are running (such as setting breakpoints), which was the point of switching to /proc/pid/mem all the time.  I would hope that with recent enough kernels things would just work reasonably.

Unfortunately, writes to /proc/pid/mem has been disabled for a long time as part of security hardening as it has been used in security expolits. Please feel free to check the bugs https://bugs.chromium.org/p/chromium/issues/detail?id=766253 and https://bugs.chromium.org/p/chromium/issues/detail?id=781376 . 

Ptrace is also discussed in that bug. It is lesser of an issue because the devices use seccomp filters for most critical and do not allow ptrace syscall to be used.


```
sure, if you have ptrace scope on a process, it def leaves it open to arbitrary read/writes of any memory region.  the difference there is that it requires you to be within a specific scope (as you noted), and you need to be able to access the ptrace syscall.  that's a much higher barrier than accessing the "mem" file which can be hit with a simple `dd` fork+exec.

when it comes to seccomp filters, we have the ability to run daemons/processes through restrictive syscall filters, and we have yet to include ptrace there.  off the top of my head, the only valid use of ptrace on the system is in upstart itself (which uses it for process tracking).  it might be interesting to severely restrict ptrace so only pid 1 (init) is allowed to use it.
```

So your kernel hardening has a bug, which you should fix.

Restricting /proc/pid/mem by default might make sense, but not if the thread that opens the mem file has ptrace permissions over pid.  If the caller is already ptracing the target process, then it already has access to all the inferior's memory.  If you want to block ptrace, then that's a different story.

The kernel already has some connection between ptrace permissions and /proc/pid/mem.  From man proc:

~~~
       /proc/[pid]/mem
              This file can be used to access the pages of a process's memory through open(2), read(2), and lseek(2).

              Permission to access this file is governed by a ptrace access mode PTRACE_MODE_ATTACH_FSCREDS check; see ptrace(2).
~~~

You would extend these checks to check whether the calling thread is a ptracer of the target pid.  Somewhere in fs/proc/base.c:mem_open and friends.  

Actually, Mike's patch already touched exactly that code back then, doing:

 static ssize_t mem_write(struct file *file, const char __user *buf,
 			 size_t count, loff_t *ppos)
 {
+#ifdef CONFIG_SECURITY_CHROMIUMOS_READONLY_PROC_SELF_MEM
+	return -EACCES;
+#else
 	return mem_rw(file, (char __user*)buf, count, ppos, 1);
+#endif
 }

so you'd just tweak that -EACESS code to first check whether the caller is a ptracer of the target, and if so, continue on with the mem_rw path.

In Mike's commit log, you've already admited "Getting this merged upstream seems unlikely", so this is all local changes to you.

Thanks for the detailed reply,
I'll forward your comments to the security team and see what do they have to say.

Update:
The security will look into relaxing the config but it will be a while. Meanwhile, we are reverting back to gdb 9.2.

Thanks for the update.  I'm glad to hear that.

Since there's nothing for us to do here, I'm going to close this for now.

Hello, sorry for necro-bumping this, but I'm working on Chromebooks on behalf of Collabora and encountered this problem - of not having a functional remote debugger past GDB 9.2 (yes CrOS based systems are still using v9.2...).

I got GDB working again by creating a kernel patch as you suggested above, to relax the /proc/pid/mem hardening restriction for ptrace-scoped processes, but it got rejected by ChromiumOS upstream as too risky.

Please see the attached kernel patch and this reply [1] if you want more details on that.

I also wrote a patch for gdbserver which adds back the the ptrace r/w as a fallback which works nicely for my test cases. I've attached the patch and tried to explain as best I can in the commit message.

Please consider the perspective of ChromeOS/ChromiumOS users, for which users GDB has a regression because basic functionality used to work and is broken in more recent releases.

Would the gdbserver patch be acceptable to fix this case? I can modify it if necessary to make it acceptable, for example after writing it I noticed that gdb/linux-nat.c contains proc_mem_file_is_writable () which does something similar, maybe I could reuse that logic?

Thanks,
Adrian 

[1] https://issuetracker.corp.google.com/issues/268356054#comment61

Created attachment 15368 [details]
Kernel patch

Created attachment 15369 [details]
gdbserver patch

> for example after writing it I noticed that gdb/linux-nat.c contains proc_mem_file_is_writable () which does something similar, maybe I could reuse that logic?

Hm.. actually I'm not that certain we can reuse that logic because it's writing to /proc/self/mem, which won't work for gdbserver IIUC.

"too risky", why?  That position doesn't make any sense to me.

But I can't open:

> [1] https://issuetracker.corp.google.com/issues/268356054#comment61

Does one have to be inside the google corporate network to open that?

Not being able to write to memory while the inferior is running is something that I'm really really REALLY not looking forward to having to support.  Having to work around an unreasoable downstream kernel change really is not appealing.

As I said, I can't access the URL, but I can't imagine how the position could be reasonable.  There's no risk in allowing a ptracer to access inferior memory via /proc/pid/mem, because it can't already do so.  So they're basically trying to push on us to go through contortions around a crutial interface.  I don't see that happening, frankly.

I meant "because it already CAN do so", of course.

> sorry for necro-bumping this

Never a problem IMO :)

> GDB 9.2

As a workaround you can use an old gdbserver and a new gdb.
I believe this should work fine.  It's maybe only mildly
inconvenient if you use the gdbserver pipe feature
("target remote | gdbserver - ...")

(In reply to Pedro Alves from comment #12)
> "too risky", why?  That position doesn't make any sense to me.
> 
> But I can't open:
> 
> > [1] https://issuetracker.corp.google.com/issues/268356054#comment61
> 
> Does one have to be inside the google corporate network to open that?

Sorry I messed up the link, here is the correct one:

https://issuetracker.google.com/u/2/issues/268356054#comment61

the relevant part from that comment, referring to the attached kernel patch is:

```
the kernel side needs a bit more thought. i understand the argument being made, but focusing purely on ACLs seems to discount the complexity of the attack. reading/writing to mem can be abused via arbitrary shell code execution, and is a way of bypassing verified boot (where only executable code in the rootfs is permitted). having arbitrary shell code is often a lot easier to pull off than arbitrary syscalls which is what ptrace() access requires. our verified boot story makes arbitrary code exec much more difficult even if arbitrary shell code exec is possible (although that is something we've worked on restricting too with some success). i don't know that, in CrOS, we'd ever want to allow writes to /proc/self/mem even if ptrace() is wide open.
```

(In reply to Tom Tromey from comment #15) 
> As a workaround you can use an old gdbserver and a new gdb.
> I believe this should work fine.  It's maybe only mildly
> inconvenient if you use the gdbserver pipe feature
> ("target remote | gdbserver - ...")

Thanks, I also tested gdbserver 9.2 together with gdb 13.1 and it works as well for the basic use cases I'm interested in.

So basically I have 3 solutions:

1. Apply the kernel patch to relax ChromeOS /proc/pid/mem policy.

2. Apply the gdbserver patch to fallback to ptrace R/W.

3. Use old gdbserver with newer gdb.

If neither of the upstreams don't want the respective patches, then maybe the path forward is to carry a change/patch for points 2 or 3 in the CrOS gdb ebuild?

At least that would give us a functional debugger without forcing the upstreams to accept a patch they don't want :)

I would like to point out one more thing, which I found interesting on this subject.

According to [1], a very long time ago the upstream Linux kernel used to restrict writes to /proc/pid/mem similar to how ChromeOS is doing in their kernel.

Eventually in 2011 the upstream kernel allowed writes to mem citing "there is no longer a security hazard" in commit [2], which was incorrect because CVEs and drama ensued [3], which apparently continues to this day.

Personally I can understand the case ChromeOS is making for restricting /proc/pid/mem, and I think it would even make sense to have those kconfig options in the upstream kernel.

I can't be the judge of what GDB supports or not, but perhaps if I get the kconfig options to restring /proc/pid/mem upstreamed in the mainline kernel, that would help settle the matter? :)

[1] https://github.com/torvalds/linux/blob/8b0db9db19858b08c46a84540acfd35f6e6487b8/fs/proc/base.c#L860
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=198214a7
[3] https://lwn.net/Articles/476947/

I wonder if it would be possible to change ptrace to lift
the restriction here.  Like IIUC the problem is that ptrace
can't read/write memory while the inferior is running, but
/proc/pid/mem can.  If ptrace had this capability, then using
it as a fallback seems possible.

Thanks, I've commented on the CrOS ticket.

> Eventually in 2011 the upstream kernel allowed writes to mem citing "there is no longer a security hazard" in commit [2], which was incorrect because CVEs and drama ensued [3], which apparently continues to this day.

So there was a CVE over a decade ago and it was fixed then.  Because there used to be a bug, and it is now fixed, we should still consider allowing a debugger to access memory using the interface bad?  That, doesn't compute to me.  Bugs happen, and they are fixed.  There is nothing in the interface that prevents it from being hardened, _and_ still allows debuggers to use it.

> I can't be the judge of what GDB supports or not, but perhaps if I get the kconfig 
> options to restring /proc/pid/mem upstreamed in the mainline kernel, that would help 
> settle the matter? :)

Sounds like blackmail.  :-P

> I wonder if it would be possible to change ptrace to lift
> the restriction here.  Like IIUC the problem is that ptrace
> can't read/write memory while the inferior is running, but
> /proc/pid/mem can.  If ptrace had this capability, then using
> it as a fallback seems possible.

/proc/pid/mem is also a lot more efficient.  The ptrace interface only lets you peek/poke one word at a time.  So a lot of context switching.  IMO, such workarounds aren't really necessary.  We're only considering them because CrOS hasn't implemented their downstream hardening properly.  But they could.  Nothing stops them.

(In reply to Pedro Alves from comment #20)
> > I can't be the judge of what GDB supports or not, but perhaps if I get the kconfig 
> > options to restring /proc/pid/mem upstreamed in the mainline kernel, that would help 
> > settle the matter? :)
> 
> Sounds like blackmail.  :-P

Thanks again and sorry again if I put unnecessary pressure. My intention is just to brainstorm solutions - that's why I wrote patches and tested all the proposed solutions until now (kernel patch, gdbserver patch, old server + new gdb).

Personally I don't have a strong opinion either way and if it keeps everyone happy I'm also open to apply a patch downstream in the CrOS specific ebuild src_prepare().

Also many thanks for engaging on the CrOS buganizer! :)

> /proc/pid/mem is also a lot more efficient.  The ptrace interface only lets you 
> peek/poke one word at a time.  So a lot of context switching. 

I should also mention, for completeness.  ptrace peek/poke requires that you pass down a pid of a live task.  So you end up with a race where the task may die while you're ptracing it, and the memory access fails.  /proc/pid/mem does not have this problem, as we open the file as soon as we start debugging the inferior, and keep it open until the inferior exits or execs (at which point we need to open another file).  We can always access /proc/pid/mem without having to worry about these cases of the thread that you pass down to ptrace vanishes.  Note the same can happen with the main thread, it can go zombie without the rest of the process exiting.  Again, /proc/pid/mem works in that corner case.

There are also the process_vm_readv and process_vm_writev syscalls. These  system  calls  were  added in Linux 3.2.  Support is provided in glibc since version 2.15. Permission  to  read  from or write to another process is governed by a ptrace access mode PTRACE_MODE_ATTACH_REALCREDS check.

https://man7.org/linux/man-pages/man2/process_vm_readv.2.html

So that is an alternative to directly reading /proc/pid/mem or using ptrace PEEK/POKE. In theory it should work if you can ptrace the process, so hopefully CrOS allows it.

We tried those before, and they don't work.  They respect page permissions, while gdb needs to be able to write to read-only pages (for breakpoints, etc.).

BTW, there's progress on the CrOS ticket.  Looks like they'll accept the kernel patch if it's fixed to check the right thing.

(though maybe the kernel could be changed to ignore permissions when the caller is a ptracer)

> ptrace peek/poke requires that you pass down a pid of a live task.

The Linux ptrace model bites again.

Hello, I have some goods news to share:

The ChromeOS developers agreed to land a version of my kernel patch to fix GDB and they also asked me to avoid building upon their existing tech debt, so I created the following patch and sent it to the upstream kernel.

Depending on how the upstream kernel discussion goes, whether they accept or reject this change, a solution will be applied to the CrOS kernels (backport or not).

I am leaving a link here to the upstream patch in case others will encounter this problem and will update if/when the patch lands in the mainline kernel. Thank you for all your support!

https://patchwork.kernel.org/project/linux-fsdevel/patch/20240221210626.155534-1-adrian.ratiu@collabora.com/

The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bd659b80301bc394f6b42d88e5fc10f944552f23

commit bd659b80301bc394f6b42d88e5fc10f944552f23
Author: Pedro Alves <pedro@palves.net>
Date:   Wed Feb 21 19:05:39 2024 +0000

    gdb/linux-nat.c: Add "Accessing inferior memory" section
    
    This commit adds a new "Accessing inferior memory" comment section to
    gdb/linux-nat.c that explains why we prefer /proc/pid/mem over
    alternatives.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30453
    
    Change-Id: I575b21ed697a85f3ff4c0ec58c04812db5005b76