Page MenuHomeFreeBSD

Avoid leaking kernel pointers from msgctl(IPC_STAT).
ClosedPublic

Authored by markj on Jul 9 2020, 2:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 18 2024, 5:14 PM
Unknown Object (File)
Dec 20 2023, 7:23 AM
Unknown Object (File)
Dec 13 2023, 7:21 AM
Unknown Object (File)
Nov 22 2023, 1:33 PM
Unknown Object (File)
Nov 22 2023, 1:25 PM
Unknown Object (File)
Nov 22 2023, 1:15 PM
Unknown Object (File)
Nov 12 2023, 9:59 AM
Unknown Object (File)
Nov 12 2023, 9:58 AM
Subscribers

Details

Summary

While this is harmless, these pointers are not useful to any user
code. Let's stop copying and documenting them.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32214
Build 29710: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jul 9 2020, 2:35 PM
markj created this revision.

I do not object but also there is no much sense in this change. Both because we do not randomize KVA and because we export KVA layouts in many ways, some of which you added.

In D25600#566343, @kib wrote:

I do not object but also there is no much sense in this change. Both because we do not randomize KVA and because we export KVA layouts in many ways, some of which you added.

I understand, it is more about the intent: we have interfaces that export kernel pointers for debugging purposes, but here it is done because the same structure is used by both userland and the kernel, i.e., the interface is poorly designed. In that case I think it is reasonable to patch the leak, if only to avoid seeing the same reports again and again.

In that case I think it is reasonable to patch the leak, if only to avoid seeing the same reports again and again.

Agreed.

This revision is now accepted and ready to land.Jul 9 2020, 2:57 PM
In D25600#566343, @kib wrote:

I do not object but also there is no much sense in this change. Both because we do not randomize KVA and because we export KVA layouts in many ways, some of which you added.

I understand, it is more about the intent: we have interfaces that export kernel pointers for debugging purposes, but here it is done because the same structure is used by both userland and the kernel, i.e., the interface is poorly designed. In that case I think it is reasonable to patch the leak, if only to avoid seeing the same reports again and again.

Then perhaps add a comment at the place that nullifies the pointers explaining why.

BTW, a serious places that export kptrs do that not for debugging. The cases that I can remember immediately are kinfo_proc with pointers to struct proc/thread and wait channels.

In D25600#566353, @kib wrote:
In D25600#566343, @kib wrote:

I do not object but also there is no much sense in this change. Both because we do not randomize KVA and because we export KVA layouts in many ways, some of which you added.

I understand, it is more about the intent: we have interfaces that export kernel pointers for debugging purposes, but here it is done because the same structure is used by both userland and the kernel, i.e., the interface is poorly designed. In that case I think it is reasonable to patch the leak, if only to avoid seeing the same reports again and again.

Then perhaps add a comment at the place that nullifies the pointers explaining why.

BTW, a serious places that export kptrs do that not for debugging. The cases that I can remember immediately are kinfo_proc with pointers to struct proc/thread and wait channels.

Even there, it is not an accident. ps even has "-o paddr" for displaying such pointers. I've used it in the past to debug ptrace issues.

This revision now requires review to proceed.Jul 9 2020, 3:11 PM
This revision is now accepted and ready to land.Jul 9 2020, 3:18 PM