limine-bootloader/limine-rs

The stored `LimineTerminalWrite` function can be used in invalid ways.

Closed this issue · 1 comments

Currently, (and even when fixing #14), it is possible to call the LimineTerminalWrite function on two separate threads with the same LimineTerminal instance.

That one is pretty simple. The function returns by LimineTerminalResponse::write method returns a function that takes a shared &LimineTerminal, meaning that the function can be called simultaneously with the same aliased terminal.

Using the same exact trick, it's possible to call the write within itself in the terminal callback implementation.

Both of those operations are invalid (as specified in the specification).

An easy way to fix this would be to require the write function to take an exclusive reference to the target terminal.

Going from:

fn write(&self) -> Option<impl Fn(&Terminal, s: &str)>;

to:

fn write(&self) -> Option<impl Fn(&mut Terminal, s: &str)>;

Note that this would require an additional terminals_mut method to be added to the LimineTerminalResponse.

Also: is there any way to remove the Option here? I mean, does it even make sense for this function not to be present?

It appears the 'Terminal' feature of the Limine protocol is being deprecated, so I'm not sure this issue will maintain relevancy for the upcoming releases. Perhaps a note could be added to the current deprecation notices that indicates the API is unsafe via shared mutable aliasing?