serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Kernel: Implement "negative" pledges

Open Flying-Toast opened this issue 1 year ago • 5 comments

In programs with multiple pledge() calls it is common to first pledge a broad initial set of promises and then progressively drop promises as they are no longer needed. Previously the only way to do this was to re-pledge() a subset of the old promises, but this requires one to re-specify all but the forfeited promises.

With this commit, we introduce the concept of "negative" pledges, for instance:

// Initial pledge
pledge("stdio rpath wpath id", NULL);

// Stuff that needs "id" here...

// Forfeit id
pledge("-id", NULL);

// Stuff that needs rpath/wpath here...

// Forfeit rpath and wpath
pledge("-rpath -wpath", NULL);

// Only stdio remains pledged at this point.

Flying-Toast avatar Feb 28 '24 17:02 Flying-Toast

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

BuggieBot avatar Feb 28 '24 17:02 BuggieBot

Hi there! Welcome to the project. :^)

What you've done here looks well put together, so good job on that, but I'm not sure it's a direction we want to go in. The way pledge currently works, it's more work to maintain the pledge lists, but it's clear, explicit, and easier to determine what the pledges are when reading the code – just look at the most recent pledge call. I understand that's also an intentional design decision by OpenBSD, where we got the idea for pledge from.

That's not to say our existing pledge is perfect, but it's not clear to me if this is an improvement or not.

AtkinsSJ avatar Feb 28 '24 22:02 AtkinsSJ

Yeah, that makes sense. I was mostly just hacking around and figured I'd polish it up and send in a PR to see what people think, but I do agree that it's better to be explicit about exactly what you can do rather than what you can't. I suppose we should close this PR - I'll hopefully be back later with some more useful contributions in the future :-)

Flying-Toast avatar Feb 28 '24 22:02 Flying-Toast

Hmm, I tried to do this similarly here: https://github.com/supercomputer7/serenity/commit/ec7992ee8037c9e22ea3f914d884b003b0cd81a1 as part of #22968. I think "negative" pledging has its use cases which worth discussing, but I don't like the fact that you semantically do this (via the minus sign). My implementation is much more concise imho, and is less prone to errors.

supercomputer7 avatar Feb 29 '24 19:02 supercomputer7

@supercomputer7 Now I'm wondering if it could all just be done in userland? A library could provide a pledge() wrapper that remembers what you've pledged so far, and expose a removal API that will pledge() with it's remembered list excluding any pledges to remove.

Flying-Toast avatar Feb 29 '24 20:02 Flying-Toast

@supercomputer7 Now I'm wondering if it could all just be done in userland? A library could provide a pledge() wrapper that remembers what you've pledged so far, and expose a removal API that will pledge() with it's remembered list excluding any pledges to remove.

That's actually better. But it should be used with some userland code, otherwise we don't really want this feature.

supercomputer7 avatar Mar 23 '24 03:03 supercomputer7