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

userspace: easy checking for specific driver #12056

Merged

Conversation

andrewboie
Copy link
Contributor

In general driver system calls are implemented at a subsystem
layer. However, some drivers may have capabilities specific to
the hardware not covered by the subsystem API. Such drivers may
want to define their own system calls.

This macro makes it simple to validate in the driver-specific
system call handlers that not only does the untrusted device
pointer correspond to the expected subsystem, initialization
state, and caller permissions, but also that the device object
is an instance of a specific driver (and not just any driver in
that subsystem).

Signed-off-by: Andrew Boie andrew.p.boie@intel.com

In general driver system calls are implemented at a subsystem
layer. However, some drivers may have capabilities specific to
the hardware not covered by the subsystem API. Such drivers may
want to define their own system calls.

This macro makes it simple to validate in the driver-specific
system call handlers that not only does the untrusted device
pointer correspond to the expected subsystem, initialization
state, and caller permissions, but also that the device object
is an instance of a specific driver (and not just any driver in
that subsystem).

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems plausible, but I won't have a chance to test it until next week.

@codecov-io
Copy link

Codecov Report

Merging #12056 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #12056   +/-   ##
=======================================
  Coverage   48.05%   48.05%           
=======================================
  Files         281      281           
  Lines       43414    43414           
  Branches    10404    10404           
=======================================
  Hits        20862    20862           
  Misses      18403    18403           
  Partials     4149     4149

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 936d8bd...fd71011. Read the comment docs.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for doc changes

@andrewboie
Copy link
Contributor Author

recheck

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. Better than ioctl().

a provided pointer is a valid instance of a specific device driver, that
the calling thread has permissions on it, and that the driver has been
initialized. It does this by checking the init function pointer that
is stored within the driver instance and ensuring that it matches the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by ensuring?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "by ensuring" would be correct. The ensuring is part of the checking that's already under "by". Perhaps:

It does this by checking that the init function pointer stored within the driver instance matches the provided value, which should be the address of the specific driver's init function.

Copy link
Collaborator

@agross-oss agross-oss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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

Successfully merging this pull request may close these issues.

9 participants