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

clk: Disable writable debugfs files #70

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

MichaIng
Copy link

@MichaIng MichaIng commented Mar 5, 2023

Revert commit friendlyarm@861a024 which may be safe for Android, but is not safe for Linux, but leads to a prominent warning at boot:

********************************************************************
**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE           **
**                                                                **
**  WRITEABLE clk DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL **
**                                                                **
** This means that this kernel is built to expose clk operations  **
** such as parent or rate setting, enabling, disabling, etc.      **
** to userspace, which may compromise security on your system.    **
**                                                                **
** If you see this message and you are not debugging the          **
** kernel, report this immediately to your vendor!                **
**                                                                **
**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE           **
********************************************************************

Also, the flag is meant for testing and debugging (Android), not for end user kernel builds.

For reference: friendlyarm#6 and #69

The other 5.10 branches have this unset already.

Revert commit friendlyarm@861a024 which may be safe for Android, but is not safe for Linux, but leads to a prominent warning at boot:

********************************************************************
**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE           **
**                                                                **
**  WRITEABLE clk DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL **
**                                                                **
** This means that this kernel is built to expose clk operations  **
** such as parent or rate setting, enabling, disabling, etc.      **
** to userspace, which may compromise security on your system.    **
**                                                                **
** If you see this message and you are not debugging the          **
** kernel, report this immediately to your vendor!                **
**                                                                **
**     NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE           **
********************************************************************

Also, the flag is meant for testing and debugging (Android), not for end user kernel builds.

For reference: friendlyarm#6
@RadxaYuntian
Copy link
Member

Other than what was mentioned in #69 (comment), this branch is also deprecated by linux-5.10-gen-rkr3.4.

@MichaIng
Copy link
Author

MichaIng commented Mar 6, 2023

Okay. That is not obvious from the branch naming. Keep in mind that others use your repository as basis for their own kernel builds and may not be aware of the this branch deprecation. So probably it makes sense to remove it then? Better to break others tools, forcing them to check, then leaving them unknowingly on a deprecated branch?

@RadxaYuntian
Copy link
Member

Very soon we will update the kernel yet again. I don't think our customers and community developers will appreciate their build process is constantly breaking.

@MichaIng
Copy link
Author

MichaIng commented Mar 6, 2023

Well, practically their build process is constantly broken anyway, as long as you do not keep updating on the same branch. But it is just done unknowingly as their used branch will be deprecated, which is worse than a broken pipeline, IMO, from security perspective.

Why not keep updating the same branch?

@RadxaYuntian
Copy link
Member

Mostly for commercial customers who pin to some specific commit. When we update kernel from one Rockchip release to another update, we have to rebase our own commits, which means the commit hash they relied on is now gone.

@MichaIng
Copy link
Author

MichaIng commented Mar 6, 2023

I see, and rebasing is cleaner than merging from Rockchip. Probably renaming the old branch to keep the commit hash valid would be another option.

However, just wanted to make the point clear that we see other distros shipping kernel builds based on your repo with the problematic writable debugfs flag. Removing this right in the source code (outside of patches) in all affected branches would rule that out, as fast as distributors bump their possibly used specific commit.

@RadxaStephen RadxaStephen merged commit cdf219e into radxa:stable-5.10-rock5 Jun 19, 2024
@MichaIng MichaIng deleted the patch-2 branch June 19, 2024 11:15
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.

None yet

3 participants