Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault due to unknown xcb::damage::ReportLevel value #228

Open
Maxdamantus opened this issue May 12, 2023 · 2 comments
Open

Segfault due to unknown xcb::damage::ReportLevel value #228

Maxdamantus opened this issue May 12, 2023 · 2 comments

Comments

@Maxdamantus
Copy link

xcb = { version = "1.2.1", features = ["damage"] }

The ReportLevel enum is only defined for values 0, 1, 2, 3, but the server will sometimes set an extra bit (DamageNotifyMore, 0x80) to denote that there are more damage events to follow.

Sample reproduction code (usually segfaults for me within a couple of seconds of updates, though I haven't tried with other setups involving compositors etc):

fn main() {
	let (conn, screen_num) = xcb::Connection::connect_with_extensions(None, &[xcb::Extension::Damage], &[]).unwrap();
	let root_window = conn.get_setup().roots().nth(0).unwrap().root();
	conn.wait_for_reply(conn.send_request(&xcb::damage::QueryVersion {
		client_major_version: xcb::damage::MAJOR_VERSION,
		client_minor_version: xcb::damage::MINOR_VERSION,
	})).unwrap();
	conn.check_request(conn.send_request_checked(&xcb::damage::Create {
		damage: conn.generate_id(),
		drawable: xcb::x::Drawable::Window(root_window),
		level: xcb::damage::ReportLevel::RawRectangles,
	})).unwrap();
	loop {
		let e = conn.wait_for_event().unwrap();
		println!("e = {:#?}", e);
	}
}
@Maxdamantus
Copy link
Author

Just thought I'd note some observations I've made:

While trying with a compositor (particularly, xcompmgr), the issue doesn't occur, because something seems to combine multiple damages into a single bounding box.

I'm not too familiar with the semantics of xcbproto, but the definition for the event (https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/blob/master/src/damage.xml) does indeed seem to omit this detail. Maybe it's an xcbproto bug?

Even if it is an xcbproto bug, is it expected that this library introduce memory issues due to unexpected data from the server? I'm guessing std::mem::transmute is used for all enums, but this seems a bit unsafe since afaik Rust assumes that the Rust enum members cover all values when performing exhaustiveness checks. I'm not sure what guarantees the xcb C API provides, but at least in this case it's exposed as a C enum type, which is of course just a regular integer type, so values are not limited to the ones listed.

@rtbo
Copy link
Collaborator

rtbo commented May 16, 2023

Hi,
In the XML documentation, the 0x80 bit is mentionned in the Notify event, but it's not in the type definition.
The thing is that ReportLevel is not really a bitfield. It is an enum with 4 possible values (RawRectangles as value 0 which can't be expressed in a bit field). It is only in the Notify event that we can have an additional bit set on top of this value.
The easiest way to deal with this IMO is to hand-write a definition for ReportLevel according the semantics I've just described and to bypass it in the code generation.
It was already done for xinput::Device

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants