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

Add ability to specify a script's license #3157

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ and this project adheres to
- [#3046](https://github.com/bpftrace/bpftrace/pull/3046)
- Add for-each loops for iterating over map elements
- [#3003](https://github.com/bpftrace/bpftrace/pull/3003)
- Add ability to specify a script's license
- [#3157](https://github.com/bpftrace/bpftrace/pull/3157)
#### Changed
- Better error message for args in mixed probes
- [#3047](https://github.com/bpftrace/bpftrace/pull/3047)
Expand Down
22 changes: 22 additions & 0 deletions man/adoc/bpftrace.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ List all the probes attached in the program::

x86_64, arm64, s390x, arm32, loongarch64, mips64, ppc64, riscv64

== Script Licenses

The license of a bpftrace script affects how it can be run, as the Linux kernel limits the functionality available to non-GPL licensed programs.

A license can be specified by setting the <<license>> config variable or by including a single-line comment containing a SPDX License ID:

----
// SPDX-License-Identifier: GPL-2.0-or-later
----

If a license is not explicitly declared, bpftrace will assume that the script is GPL v2 compatible.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This language feels a little weird to me. How about "If a license is not explicitly declared, bpftrace will license the script as GPL v2."


== Options

=== *-B* _MODE_
Expand Down Expand Up @@ -3340,6 +3352,16 @@ Default: 0

For user space symbols, symbolicate lazily/on-demand (1) or symbolicate everything ahead of time (0).

=== license

Default: <unset>

The license covering the script's source code.

This license string is passed to the kernel where it is used to determine what functionality can be used.
ajor marked this conversation as resolved.
Show resolved Hide resolved

Licenses recognised by the Linux kernel are listed in link:https://docs.kernel.org/process/license-rules.html#id1[its documentation].

=== log_size

Default: 1000000
Expand Down
14 changes: 6 additions & 8 deletions src/ast/irbuilderbpf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,14 +607,12 @@ CallInst *IRBuilderBPF::CreateProbeReadStr(Value *ctx,
// int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
FunctionType *probereadstr_func_type = FunctionType::get(
getInt64Ty(), { dst->getType(), getInt32Ty(), src->getType() }, false);
PointerType *probereadstr_func_ptr_type = PointerType::get(
probereadstr_func_type, 0);
Constant *probereadstr_callee = ConstantExpr::getCast(
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I might have missed it but what is this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, you're right this could have done with an explanation. It's to produce more useful error messages for the bpf_probe_read_str helper. A lot of other helpers are already using this CreateHelperCall utility function, and then rest probably should be switched to it in the future as well.

bpf_probe_read_str is a GPL-only helper function so I ended up getting a lot of error messages with this one in particular when playing around with different licenses.

Without this change:

# bpftrace -e 'config = {license = "poo"; } BEGIN { str(0); }' 
WARNING: Addrspace is not set
Attaching 1 probe...
ERROR: helper bpf_probe_read_str can only be used in GPL-compatible programs

With this change:

# bpftrace -e 'config = {license = "poo"; } BEGIN { str(0); }' 
WARNING: Addrspace is not set
Attaching 1 probe...
stdin:1:38-44: ERROR: helper bpf_probe_read_str can only be used in GPL-compatible programs
config = {license = "poo"; } BEGIN { str(0); }
                                     ~~~~~~

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah nice. Thanks for the explanation.

Instruction::IntToPtr, getInt64(read_fn), probereadstr_func_ptr_type);
CallInst *call = createCall(probereadstr_func_type,
probereadstr_callee,
{ dst, size_i32, src },
probeReadHelperName(read_fn));

CallInst *call = CreateHelperCall(read_fn,
probereadstr_func_type,
{ dst, size_i32, src },
probeReadHelperName(read_fn),
&loc);
CreateHelperErrorCond(ctx, call, read_fn, loc);
return call;
}
Expand Down
6 changes: 5 additions & 1 deletion src/ast/passes/codegen_llvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,16 @@ CodegenLLVM::CodegenLLVM(Node *root, BPFtrace &bpftrace)
llvm::DEBUG_METADATA_VERSION);

// Set license of BPF programs
const std::string license = "GPL";
const auto &license = bpftrace_.get_license();
auto license_var = llvm::dyn_cast<GlobalVariable>(module_->getOrInsertGlobal(
"LICENSE", ArrayType::get(b_.getInt8Ty(), license.size() + 1)));
license_var->setInitializer(
ConstantDataArray::getString(module_->getContext(), license.c_str()));
license_var->setSection("license");

// XXX: Set licenses for manually loaded programs. Remove once libbpf is used
jordalgo marked this conversation as resolved.
Show resolved Hide resolved
// instead:
bpftrace_.resources.license = license;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's just temporary but this doesn't feel like a good place to set this. Why not do it in ResourceAnalyser?

}

void CodegenLLVM::visit(Integer &integer)
Expand Down
17 changes: 11 additions & 6 deletions src/attached_probe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,11 @@ AttachedProbe::AttachedProbe(Probe &probe,
BpfProgram &&prog,
bool safe_mode,
BPFfeature &feature,
BTF &btf)
BTF &btf,
cstring_view license)
: probe_(probe), prog_(std::move(prog)), btf_(btf)
{
load_prog(feature);
load_prog(feature, license);
LOG(V1) << "Attaching " << probe_.orig_name;
switch (probe_.type) {
case ProbeType::special:
Expand Down Expand Up @@ -228,10 +229,11 @@ AttachedProbe::AttachedProbe(Probe &probe,
int pid,
BPFfeature &feature,
BTF &btf,
cstring_view license,
bool safe_mode)
: probe_(probe), prog_(std::move(prog)), btf_(btf)
{
load_prog(feature);
load_prog(feature, license);
switch (probe_.type) {
case ProbeType::usdt:
attach_usdt(pid, feature);
Expand Down Expand Up @@ -698,14 +700,13 @@ void maybe_throw_helper_verifier_error(std::string_view log,
}
}

void AttachedProbe::load_prog(BPFfeature &feature)
void AttachedProbe::load_prog(BPFfeature &feature, cstring_view license)
{
if (use_cached_progfd())
return;

auto &insns = prog_.getCode();
auto func_infos = prog_.getFuncInfos();
const char *license = "GPL";
int log_level = 0;

uint64_t log_buf_size = probe_.log_size;
Expand Down Expand Up @@ -834,7 +835,7 @@ void AttachedProbe::load_prog(BPFfeature &feature)
if (btf_fd >= 0) {
progfd_ = bpf_prog_load(static_cast<::bpf_prog_type>(prog_type),
name.c_str(),
license,
license.c_str(),
reinterpret_cast<const struct bpf_insn *>(
insns.data()),
insns.size() / sizeof(struct bpf_insn),
Expand Down Expand Up @@ -866,6 +867,10 @@ void AttachedProbe::load_prog(BPFfeature &feature)
maybe_throw_helper_verifier_error(log_buf.get(),
"helper call is not allowed in probe",
" not allowed in probe");
maybe_throw_helper_verifier_error(
log_buf.get(),
"cannot call GPL-restricted function from non-GPL compatible program",
" can only be used in GPL-compatible programs");

std::stringstream errmsg;
errmsg << "Error loading program: " << probe_.name
Expand Down
7 changes: 5 additions & 2 deletions src/attached_probe.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "bpffeature.h"
#include "bpfprogram.h"
#include "btf.h"
#include "container/cstring_view.h"
#include "types.h"

#include <bcc/libbpf.h>
Expand All @@ -24,12 +25,14 @@ class AttachedProbe {
BpfProgram &&prog,
bool safe_mode,
BPFfeature &feature,
BTF &btf);
BTF &btf,
cstring_view license);
AttachedProbe(Probe &probe,
BpfProgram &&prog,
int pid,
BPFfeature &feature,
BTF &btf,
cstring_view license,
bool safe_mode = true);
~AttachedProbe();
AttachedProbe(const AttachedProbe &) = delete;
Expand All @@ -44,7 +47,7 @@ class AttachedProbe {
std::string eventname() const;
void resolve_offset_kprobe(bool safe_mode);
bool resolve_offset_uprobe(bool safe_mode);
void load_prog(BPFfeature &feature);
void load_prog(BPFfeature &feature, cstring_view license);
void attach_multi_kprobe(void);
void attach_multi_uprobe(int pid);
void attach_kprobe(bool safe_mode);
Expand Down
45 changes: 36 additions & 9 deletions src/bpftrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ std::vector<std::unique_ptr<AttachedProbe>> BPFtrace::attach_usdt_probe(
if (feature_->has_uprobe_refcnt() ||
!(file_activation && probe.path.size())) {
ret.emplace_back(std::make_unique<AttachedProbe>(
probe, std::move(program), pid, *feature_, *btf_));
probe, std::move(program), pid, *feature_, *btf_, resources.license));
return ret;
}

Expand Down Expand Up @@ -822,8 +822,12 @@ std::vector<std::unique_ptr<AttachedProbe>> BPFtrace::attach_usdt_probe(
LOG(FATAL) << "failed to parse pid=" << pid_str;
}

ret.emplace_back(std::make_unique<AttachedProbe>(
probe, std::move(program), pid_parsed, *feature_, *btf_));
ret.emplace_back(std::make_unique<AttachedProbe>(probe,
std::move(program),
pid_parsed,
*feature_,
*btf_,
resources.license));
break;
}
}
Expand Down Expand Up @@ -891,17 +895,30 @@ std::vector<std::unique_ptr<AttachedProbe>> BPFtrace::attach_probe(
return ret;
} else if (probe.type == ProbeType::uprobe ||
probe.type == ProbeType::uretprobe) {
ret.emplace_back(std::make_unique<AttachedProbe>(
probe, std::move(*program), pid, *feature_, *btf_, safe_mode_));
ret.emplace_back(std::make_unique<AttachedProbe>(probe,
std::move(*program),
pid,
*feature_,
*btf_,
resources.license,
safe_mode_));
return ret;
} else if (probe.type == ProbeType::watchpoint ||
probe.type == ProbeType::asyncwatchpoint) {
ret.emplace_back(std::make_unique<AttachedProbe>(
probe, std::move(*program), pid, *feature_, *btf_));
ret.emplace_back(std::make_unique<AttachedProbe>(probe,
std::move(*program),
pid,
*feature_,
*btf_,
resources.license));
return ret;
} else {
ret.emplace_back(std::make_unique<AttachedProbe>(
probe, std::move(*program), safe_mode_, *feature_, *btf_));
ret.emplace_back(std::make_unique<AttachedProbe>(probe,
std::move(*program),
safe_mode_,
*feature_,
*btf_,
resources.license));
return ret;
}
} catch (const EnospcException &e) {
Expand Down Expand Up @@ -2293,4 +2310,14 @@ struct bcc_symbol_option &BPFtrace::get_symbol_opts()
return symopts;
}

std::string BPFtrace::get_license() const
{
const auto &opt_license = config_.try_get(ConfigKeyString::license);
if (opt_license)
return *opt_license;

LOG(V1) << "No license detected... defaulting to GPL";
return "GPL";
}

} // namespace bpftrace
1 change: 1 addition & 0 deletions src/bpftrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ class BPFtrace {
bool has_btf_data() const;
Dwarf *get_dwarf(const std::string &filename);
Dwarf *get_dwarf(const ast::AttachPoint &attachpoint);
std::string get_license() const;

std::string cmd_;
bool finalize_ = false;
Expand Down
1 change: 1 addition & 0 deletions src/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Config::Config(bool has_cmd, bool bt_verbose) : bt_verbose_(bt_verbose)
config_map_ = {
{ ConfigKeyBool::cpp_demangle, { .value = true } },
{ ConfigKeyBool::lazy_symbolication, { .value = false } },
{ ConfigKeyString::license, { .value = std::nullopt } },
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit strange to me. The default license is GPL but we set the default config value to none here and only return "GPL" from BPFtrace::get_license. Why not set it to "GPL" here and only override the value from SPDX/config, if specified?

If the only reason is that we don't have a config priority defined for config value coming from a script comment header (SPDX here), then I believe that we should introduce that. It will also save the introduction of std::optional<T> Config::try_get.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Thank you @viktormalik for articulating this. I also didn't understand why we didn't want to set the default in the config.

{ ConfigKeyInt::log_size, { .value = (uint64_t)1000000 } },
{ ConfigKeyInt::max_bpf_progs, { .value = (uint64_t)512 } },
{ ConfigKeyInt::max_cat_bytes, { .value = (uint64_t)10240 } },
Expand Down
27 changes: 26 additions & 1 deletion src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ enum class ConfigKeyInt {
};

enum class ConfigKeyString {
license,
str_trunc_trailer,
};

Expand All @@ -63,6 +64,7 @@ const std::map<std::string, ConfigKey> CONFIG_KEY_MAP = {
{ "cache_user_symbols", ConfigKeyUserSymbolCacheType::default_ },
{ "cpp_demangle", ConfigKeyBool::cpp_demangle },
{ "lazy_symbolication", ConfigKeyBool::lazy_symbolication },
{ "license", ConfigKeyString::license },
{ "log_size", ConfigKeyInt::log_size },
{ "max_bpf_progs", ConfigKeyInt::max_bpf_progs },
{ "max_cat_bytes", ConfigKeyInt::max_cat_bytes },
Expand All @@ -83,7 +85,12 @@ const std::set<std::string> ENV_ONLY = {

struct ConfigValue {
ConfigSource source = ConfigSource::default_;
std::variant<bool, uint64_t, std::string, StackMode, UserSymbolCacheType>
std::variant<bool,
uint64_t,
std::string,
StackMode,
UserSymbolCacheType,
std::nullopt_t>
value;
};

Expand All @@ -106,6 +113,11 @@ class Config {
return get<std::string>(key);
}

std::optional<std::string> try_get(ConfigKeyString key) const
{
return try_get<std::string>(key);
}

StackMode get(ConfigKeyStackMode key) const
{
return get<StackMode>(key);
Expand Down Expand Up @@ -154,6 +166,19 @@ class Config {
}
}

template <typename T>
std::optional<T> try_get(ConfigKey key) const
{
auto it = config_map_.find(key);
if (it == config_map_.end()) {
throw std::runtime_error("Config key does not exist in map");
}
auto *p = std::get_if<T>(&it->second.value);
if (p)
return *p;
return {};
}

private:
bool can_set(ConfigSource prevSource, ConfigSource);
bool is_aslr_enabled();
Expand Down
31 changes: 31 additions & 0 deletions src/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,31 @@ extern bpftrace::location loc;

namespace bpftrace {

namespace {
std::string_view parse_license()
{
std::string_view program = Log::get().get_source();
constexpr std::string_view spdx_id_marker = "SPDX-License-Identifier: ";
auto spdx_id_pos = program.find(spdx_id_marker);
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

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

Should we ensure this is inside a comment? Seems like this would match this string anywhere in the file

if (spdx_id_pos == program.npos)
return {};

auto license_pos = spdx_id_pos + spdx_id_marker.size();
auto eol_pos = program.find("\n", spdx_id_pos + 1);
if (eol_pos == program.npos)
return {};

auto license = program.substr(license_pos, eol_pos - license_pos);

// Try to translate some known GPL-v2-compatible licenses into a format
// understood by the Linux kernel
if (license.find("GPL-2.0") == 0)
ajor marked this conversation as resolved.
Show resolved Hide resolved
return "GPL";
jordalgo marked this conversation as resolved.
Show resolved Hide resolved

return license;
}
} // namespace

Driver::Driver(BPFtrace &bpftrace, std::ostream &o)
: bpftrace_(bpftrace), out_(o)
{
Expand All @@ -31,6 +56,12 @@ int Driver::parse_str(std::string_view script)

int Driver::parse()
{
if (auto license = parse_license(); !license.empty()) {
LOG(V1) << "Found license from SPDX ID: " << license;
ConfigSetter config_setter(bpftrace_.config_, ConfigSource::default_);
config_setter.set(ConfigKeyString::license, std::string{ license });
}

// Reset previous state if we parse more than once
root.reset();

Expand Down
2 changes: 2 additions & 0 deletions src/required_resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class RequiredResources {
void load_state(std::istream &in);
void load_state(const uint8_t *ptr, size_t len);

std::string license;

// Async argument metadata
std::vector<std::tuple<FormatString, std::vector<Field>>> system_args;
// mapped_printf_args stores seq_printf, debugf arguments
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ add_executable(bpftrace_test
mocks.cpp
output.cpp
parser.cpp
parser_licenses.cpp
portability_analyser.cpp
procmon.cpp
probe.cpp
Expand Down