Skip to content

SWSPLAT-30708 xrt_kernel validate arg offset/size against execbuf payload #9895

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.

Sign up for GitHub

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

Open
stsoe wants to merge 1 commit into
base: master
Choose a base branch
from
+72 −26
Open

SWSPLAT-30708 xrt_kernel validate arg offset/size against execbuf payload #9895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 66 additions & 19 deletions src/runtime_src/core/common/api/xrt_kernel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ class kernel_command : public xrt_core::command
using execbuf_type = xrt_core::bo_cache::cmd_bo<ert_start_kernel_cmd>;
using callback_function_type = std::function<void(ert_cmd_state)>;
using callback_list = std::vector<callback_function_type>;

static constexpr size_t allocation_size = xrt_core::bo_cache::bo_size;
private:
// Return state of underlying exec buffer packet This is an
// asynchronous call, the command object may not be in the same
Expand Down Expand Up @@ -2281,10 +2281,11 @@ class run_impl : public std::enable_shared_from_this<run_impl>
struct arg_setter : argument::setter
{
uint8_t* data;
size_t m_payload_size; // bytes available in payload starting at data

explicit
arg_setter(uint32_t* d)
arg_setter(uint32_t* d, size_t payload_size)
: data(reinterpret_cast<uint8_t*>(d))
, m_payload_size(payload_size)
{}

virtual
Expand Down Expand Up @@ -2315,39 +2316,46 @@ class run_impl : public std::enable_shared_from_this<run_impl>
// AP_CTRL_HS, AP_CTRL_CHAIN
struct hs_arg_setter : arg_setter
{
explicit
hs_arg_setter(uint32_t* data)
: arg_setter(data)
hs_arg_setter(uint32_t* data, size_t payload_size)
: arg_setter(data, payload_size)
{}

void
set_offset_value(size_t offset, const arg_range<uint8_t>& value) override
{
// max 4 bytes supported for direct register write
auto count = std::min<size_t>(4, value.size());
if (offset > m_payload_size || count > m_payload_size - offset)
throw xrt_core::error(-EINVAL, "kernel arg offset/size exceeds execbuf payload");

std::copy_n(value.begin(), count, data + offset);
}

void
set_arg_value(const argument& arg, const arg_range<uint8_t>& value) override
{
auto count = std::min(arg.size(), value.size());
if (arg.offset() > m_payload_size || count > m_payload_size - arg.offset())
throw xrt_core::error(-EINVAL, "kernel arg offset/size exceeds execbuf payload");

std::copy_n(value.begin(), count, data + arg.offset());
}

arg_range<uint8_t>
get_arg_value(const argument& arg) override
{
if (arg.offset() > m_payload_size || arg.size() > m_payload_size - arg.offset())
throw xrt_core::error(-EINVAL, "kernel arg offset/size exceeds execbuf payload");

return { data + arg.offset(), arg.size() };
}
};

// FAST_ADAPTER
struct fa_arg_setter : arg_setter
{
explicit
fa_arg_setter(uint32_t* data)
: arg_setter(data)
fa_arg_setter(uint32_t* data, size_t payload_size)
: arg_setter(data, payload_size)
{}

void
Expand All @@ -2359,7 +2367,12 @@ class run_impl : public std::enable_shared_from_this<run_impl>
void
set_arg_value(const argument& arg, const arg_range<uint8_t>& value) override
{
auto fa_offset = arg.fa_desc_offset() / sizeof(uint32_t) * sizeof(uint32_t);
auto desc = reinterpret_cast<ert_fa_descriptor*>(data);
if (fa_offset > m_payload_size - offsetof(ert_fa_descriptor, data)
|| sizeof(ert_fa_desc_entry) > m_payload_size - offsetof(ert_fa_descriptor, data) - fa_offset)
throw xrt_core::error(-EINVAL, "kernel fa_desc_offset exceeds execbuf payload");

auto desc_entry = reinterpret_cast<ert_fa_desc_entry*>(desc->data + arg.fa_desc_offset() / sizeof(uint32_t));
desc_entry->arg_offset = arg.offset();
desc_entry->arg_size = arg.size();
Expand All @@ -2370,7 +2383,12 @@ class run_impl : public std::enable_shared_from_this<run_impl>
arg_range<uint8_t>
get_arg_value(const argument& arg) override
{
auto fa_offset = arg.fa_desc_offset() / sizeof(uint32_t) * sizeof(uint32_t);
auto desc = reinterpret_cast<ert_fa_descriptor*>(data);
if (fa_offset > m_payload_size - offsetof(ert_fa_descriptor, data)
|| sizeof(ert_fa_desc_entry) > m_payload_size - offsetof(ert_fa_descriptor, data) - fa_offset)
throw xrt_core::error(-EINVAL, "kernel fa_desc_offset exceeds execbuf payload");

auto desc_entry = reinterpret_cast<ert_fa_desc_entry*>(desc->data + arg.fa_desc_offset() / sizeof(uint32_t));
return { reinterpret_cast<uint8_t*>(desc_entry->arg_value), arg.size() };
}
Expand All @@ -2379,9 +2397,8 @@ class run_impl : public std::enable_shared_from_this<run_impl>
// PS_KERNEL
struct ps_arg_setter : hs_arg_setter
{
explicit
ps_arg_setter(uint32_t* data)
: hs_arg_setter(data)
ps_arg_setter(uint32_t* data, size_t payload_size)
: hs_arg_setter(data, payload_size)
{}

void
Expand Down Expand Up @@ -2418,18 +2435,48 @@ class run_impl : public std::enable_shared_from_this<run_impl>
hwctx, ctrl_code_id, ctrlpkt_bo);
}

// payload_size() - Number of bytes in command payload that can be
// used for kernel arguments.
size_t
payload_size() const
{
// run_impl::data is offset from cmd->get_ert_cmd() computed
// during command initialization. The command buffer size is
// kernel_command::allocation_size (4K).
// Total number of bytes available for payload is size of command
// buffer minus number of bytes assigned during command
// initialization.
auto reserved = (data - cmd->get_ert_cmd<uint32_t*>()) * sizeof(uint32_t);
auto payload_size = kernel_command::allocation_size - reserved;

// If regmap_size is initialized, it holds the number of words
// used by command arguments, which must not be greater than
// the remaining payload size
if (auto regmap_size = kernel->get_regmap_size()) {
auto regmap_bytes = regmap_size * sizeof(uint32_t);
if (regmap_bytes > payload_size)
throw xrt_core::error(-EINVAL, "kernel register map exceeed command payload size");

return regmap_bytes;
}

// Without regmap_size, unlimitted number of arguments is
// supported, but must not exceeed remaining payload size
return payload_size;
}

virtual std::unique_ptr<arg_setter>
make_arg_setter()
{
switch (kernel->get_kernel_type()) {
case kernel_type::pl :
if (kernel->get_ip_control_protocol() == control_type::fa)
return std::make_unique<fa_arg_setter>(data);
return std::make_unique<hs_arg_setter>(data);
return std::make_unique<fa_arg_setter>(data, payload_size());
return std::make_unique<hs_arg_setter>(data, payload_size());
case kernel_type::ps :
return std::make_unique<ps_arg_setter>(data);
return std::make_unique<ps_arg_setter>(data, payload_size());
case kernel_type::dpu :
return std::make_unique<hs_arg_setter>(data);
return std::make_unique<hs_arg_setter>(data, payload_size());
case kernel_type::none :
throw std::runtime_error("Internal error: unknown kernel type");
}
Expand Down Expand Up @@ -3279,8 +3326,8 @@ class mailbox_impl : public run_impl
mailbox_impl* mbox;
static constexpr size_t wsize = sizeof(uint32_t); // register word size

hs_arg_setter(uint32_t* data, mailbox_impl* mimpl)
: run_impl::hs_arg_setter(data), data32(data), mbox(mimpl)
hs_arg_setter(uint32_t* data, size_t payload_size, mailbox_impl* mimpl)
: run_impl::hs_arg_setter(data, payload_size), data32(data), mbox(mimpl)
{}

void
Expand Down Expand Up @@ -3479,7 +3526,7 @@ class mailbox_impl : public run_impl
if (kernel->get_ip_control_protocol() == control_type::fa)
throw xrt_core::error("Mailbox not supported with FAST_ADAPTER");

return std::make_unique<hs_arg_setter>(data, this); // data is run_impl::data
return std::make_unique<hs_arg_setter>(data, payload_size(), this); // data is run_impl::data
}

throw xrt_core::error("Mailbox not supported for non pl kernel types");
Expand Down
13 changes: 6 additions & 7 deletions src/runtime_src/core/common/bo_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,17 @@ namespace xrt_core {
// Create a cache of CMD BO objects -- for now only used for M2M -- to reduce
// the overhead of BO life cycle management.
template <size_t BoSize>
class bo_cache_t {
class bo_cache_t

@github-actions github-actions Bot Jul 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'bo_cache_t' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class bo_cache_t
      ^

{
public:
// Helper typedef for std::pair. Note the elements are const so that the
// pair is immutable. The clients should not change the contents of cmd_bo.
template <typename CommandType>
using cmd_bo = std::pair<std::unique_ptr<buffer_handle>, CommandType *const>;
private:
// Expose bo size in bytes to clients who use default bo_cache type
static constexpr size_t bo_size = BoSize * 1024u; // NOLINT

// We are really allocating a page size as that is what xocl/zocl do. Note on
// POWER9 pagesize maybe more than 4K, xocl would upsize the allocation to the
// correct pagesize. unmap always unmaps the full page.
static constexpr size_t m_bo_size = BoSize;
private:
std::shared_ptr<device> m_device;
// Maximum number of BOs that can be cached in the pool. Value of 0 indicates
// caching should be disabled.
Expand Down Expand Up @@ -92,7 +91,7 @@ class bo_cache_t {
}
}

auto execHandle = m_device->alloc_bo(m_bo_size, XCL_BO_FLAGS_EXECBUF);
auto execHandle = m_device->alloc_bo(bo_size, XCL_BO_FLAGS_EXECBUF);
auto map = execHandle->map(buffer_handle::map_type::write);
return std::make_pair(std::move(execHandle), map);
}
Expand Down
Loading