Page MenuHomeFreeBSD

pf: Set the pfik_group for userspace
ClosedPublic

Authored by kp on May 16 2021, 10:46 AM.

Details

Summary

Userspace relies on this pointer to work out if the kif is a group or
not. It can't use it for anything else, because it's a pointer to a
kernel address. Substitute 0xdeadc0de for 'true', so that we don't leak
kernel memory addresses to userspace.

PR: 255852
MFC after: 1 week

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

0xdeadc0de is used to signal a use after free.
Please consider a different code, which does not cause confusion in the future.

0xdeadc0de is used to signal a use after free.
Please consider a different code, which does not cause confusion in the future.

That's a reasonable point. Using something else would be less likely to cause people to confuse the pointer for a freed memory area. How does 0xFEEDC0DE sound?
Or perhaps just '-1', 0xffffffff. Although using a more-or-less unique combination would probably make it easier to figure out where it came from. On the third hand, pf_kkif_to_kif() is a pretty obvious place to look.

So, absent objections, I think go for 0xFEEDC0DE.

In D30284#680331, @kp wrote:

Although using a more-or-less unique combination would probably make it easier to figure out where it came from. On the third hand, pf_kkif_to_kif() is a pretty obvious place to look.

k -> 6, i -> 1, to -> 2 => (p)fkkif(to)kif => 0xf661f261f

This revision is now accepted and ready to land.May 16 2021, 1:09 PM

k -> 6, i -> 1, to -> 2 => (p)fkkif(to)kif => 0xf661f261f

I think I prefer 0xFEEDC0DE, because it's more obviously human-generated and not a valid pointer.

In D30284#680337, @kp wrote:

k -> 6, i -> 1, to -> 2 => (p)fkkif(to)kif => 0xf661f261f

I think I prefer 0xFEEDC0DE, because it's more obviously human-generated and not a valid pointer.

LTGM.

This revision was automatically updated to reflect the committed changes.