Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Check elf header for executable #35215

Closed
wants to merge 6 commits into from

Conversation

HaoranYi
Copy link
Contributor

@HaoranYi HaoranYi commented Feb 16, 2024

Problem

Verify elf header for executable for bpf_loader programs.

Summary of Changes

Check elf header for executable

Fixes #

if account.data().is_empty() {
// First, check if the account is empty or all zeros. Empty or all zeros
// accounts are not executable.
if account.data().is_empty() || account.data().iter().all(|&x| x == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

That check is too expensive, better to test if the ELF header exists in RBPF.

Copy link
Contributor Author

@HaoranYi HaoranYi Feb 19, 2024

Choose a reason for hiding this comment

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

yeah. That sound like a better idea.

@HaoranYi HaoranYi changed the title all zeros account should not be executable Check elf header for executable Feb 20, 2024
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d223a43) 81.6% compared to head (6c989e7) 81.6%.
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #35215     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         834      834             
  Lines      224827   224846     +19     
=========================================
+ Hits       183538   183553     +15     
- Misses      41289    41293      +4     

@willhickey
Copy link
Contributor

This repository is no longer in use. Please re-open this pull request in the agave repo: https://github.com/anza-xyz/agave

@willhickey willhickey closed this Mar 3, 2024
@HaoranYi
Copy link
Contributor Author

HaoranYi commented Mar 5, 2024

Moved to anza/agave

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

Successfully merging this pull request may close these issues.

3 participants