Throw exception for invalid packets at packet streams

Some application code is dereferencing the pointer returned by
'packet_content' at packet streams without checking that it is valid.
Throw an exception rather than return a null pointer, except for
zero-length packets, which have somewhat implicit invalid content and
that we believe to be properly handled in all current cases.

The client-side of a packet stream cannot take corrective action if the
server-side is sending packets with invalid content, but the servers
that provide packet streams should catch this exception to detect
misbehaving clients.

Ref #3059
This commit is contained in:
Ehmry - 2018-11-25 11:58:58 +01:00 committed by Christian Helmuth
parent e5d1d26535
commit a2bdcc68c2
20 changed files with 100 additions and 77 deletions

View File

@ -82,7 +82,7 @@ bool Session_component::_send()
if (!_tx.sink()->packet_avail()) { return false; }
Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
warning("invalid tx packet");
return true;
}

View File

@ -497,7 +497,8 @@ class Usb::Worker : public Genode::Weak_object<Usb::Worker>
_p_in_flight++;
if (!_device || !_device->udev ||
_device->udev->state == USB_STATE_NOTATTACHED) {
_device->udev->state == USB_STATE_NOTATTACHED ||
!_sink->packet_valid(p)) {
_ack_packet(p);
continue;
}

View File

@ -82,7 +82,7 @@ bool Session_component::_send()
if (!_tx.sink()->packet_avail()) { return false; }
Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
warning("invalid tx packet");
return true;
}

View File

@ -74,8 +74,9 @@ class Nic_client
count++ < MAX_PACKETS)
{
Nic::Packet_descriptor p = _nic.rx()->get_packet();
net_driver_rx(_nic.rx()->packet_content(p), p.size());
try { net_driver_rx(_nic.rx()->packet_content(p), p.size()); }
catch (Genode::Packet_descriptor::Invalid_packet) {
Genode::error("received invalid Nic packet"); }
_nic.rx()->acknowledge_packet(p);
}

View File

@ -131,7 +131,8 @@ class Usb_nic::Session_component : public Nic::Session_component
}
/* copy packet to current data pos */
Genode::memcpy(work_skb.data, _tx.sink()->packet_content(packet), packet.size());
try { Genode::memcpy(work_skb.data, _tx.sink()->packet_content(packet), packet.size()); }
catch (Genode::Packet_descriptor::Invalid_packet) { }
/* call fixup on dummy SKB */
_device->tx_fixup(&work_skb);
/* advance to next slot */
@ -155,7 +156,7 @@ class Usb_nic::Session_component : public Nic::Session_component
return false;
Genode::Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
Genode::warning("Invalid tx packet");
return true;
}

View File

@ -482,7 +482,7 @@ class Usb::Worker : public Genode::Weak_object<Usb::Worker>
_p_in_flight++;
if (!_device || !_device->udev) {
if (!_device || !_device->udev || !_sink->packet_valid(p)) {
_ack_packet(p);
continue;
}

View File

@ -138,7 +138,7 @@ class Wifi_session_component : public Nic::Session_component
if (!_tx.sink()->packet_avail()) { return false; }
Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
warning("invalid tx packet");
return true;
}

View File

@ -65,7 +65,6 @@ class Rump_fs::Session_component : public Session_rpc_object
*/
void _process_packet_op(Packet_descriptor &packet, Open_node &open_node)
{
void * const content = tx_sink()->packet_content(packet);
size_t const length = packet.length();
/* resulting length */
@ -75,9 +74,9 @@ class Rump_fs::Session_component : public Session_rpc_object
switch (packet.operation()) {
case Packet_descriptor::READ:
if (content && (packet.length() <= packet.size())) {
res_length = open_node.node().read((char *)content, length,
packet.position());
if (tx_sink()->packet_valid(packet) && packet.length() <= packet.size()) {
res_length = open_node.node().read((char *)tx_sink()->packet_content(packet),
length, packet.position());
/* read data or EOF is a success */
succeeded = res_length || (packet.position() >= open_node.node().status().size);
@ -85,8 +84,8 @@ class Rump_fs::Session_component : public Session_rpc_object
break;
case Packet_descriptor::WRITE:
if (content && (packet.length() <= packet.size())) {
res_length = open_node.node().write((char const *)content,
if (tx_sink()->packet_valid(packet) && packet.length() <= packet.size()) {
res_length = open_node.node().write((char const *)tx_sink()->packet_content(packet),
length,
packet.position());
/* File system session can't handle partial writes */

View File

@ -106,7 +106,7 @@ class Block::Session_component : public Block::Session_component_base,
_p_to_handle.succeeded(false);
/* ignore invalid packets */
if (!packet.size() || !_range_check(_p_to_handle)) {
if (!packet.size() || !_range_check(_p_to_handle) || !tx_sink()->packet_valid(packet)) {
_ack_packet(_p_to_handle);
return;
}
@ -151,6 +151,9 @@ class Block::Session_component : public Block::Session_component_base,
_req_queue_full = true;
} catch (Driver::Io_error) {
_ack_packet(_p_to_handle);
} catch (Genode::Packet_descriptor::Invalid_packet) {
_p_to_handle = Packet_descriptor();
Genode::error("dropping invalid Block packet");
}
}

View File

@ -129,6 +129,11 @@ class Genode::Packet_descriptor
*/
enum Alignment { PACKET_ALIGNMENT = 0 };
/**
* Exception type thrown by packet streams
*/
class Invalid_packet { };
/**
* Constructor
*/
@ -530,6 +535,24 @@ class Genode::Packet_stream_base
* Return communication buffer
*/
Genode::Dataspace_capability _dataspace() { return _ds_cap; }
bool packet_valid(Packet_descriptor packet)
{
return (packet.offset() >= _bulk_buffer_offset
&& packet.offset() < _bulk_buffer_offset + (Genode::off_t)_bulk_buffer_size
&& packet.offset() + packet.size() <= _bulk_buffer_offset + _bulk_buffer_size);
}
template<typename CONTENT_TYPE>
CONTENT_TYPE *packet_content(Packet_descriptor packet)
{
if (!packet.size()) return nullptr;
if (!packet_valid(packet) || packet.size() < sizeof(CONTENT_TYPE))
throw Packet_descriptor::Invalid_packet();
return (CONTENT_TYPE *)((Genode::addr_t)_ds_local_base + packet.offset());
}
};
@ -621,6 +644,8 @@ class Genode::Packet_stream_source : private Packet_stream_base
_bulk_buffer_size);
}
using Packet_stream_base::packet_valid;
/**
* Return the size of the bulk buffer.
*/
@ -680,25 +705,14 @@ class Genode::Packet_stream_source : private Packet_stream_base
return Packet_descriptor((Genode::off_t)base, size);
}
bool packet_valid(Packet_descriptor packet)
{
return (packet.offset() >= _bulk_buffer_offset
&& packet.offset() < _bulk_buffer_offset + (Genode::off_t)_bulk_buffer_size
&& packet.offset() + packet.size() <= _bulk_buffer_offset + _bulk_buffer_size);
}
/**
* Get pointer to the content of the specified packet
*
* \return 0 if the packet is invalid
* \throw Packet_descriptor::Invalid_packet raise an exception if
the packet is invalid
*/
Content_type *packet_content(Packet_descriptor packet)
{
if (!packet_valid(packet) || packet.size() < sizeof(Content_type))
return 0;
return (Content_type *)((Genode::addr_t)_ds_local_base + packet.offset());
}
Content_type *packet_content(Packet_descriptor packet) {
return Packet_stream_base::packet_content<Content_type>(packet); }
/**
* Return true if submit queue can hold another packet
@ -786,6 +800,8 @@ class Genode::Packet_stream_sink : private Packet_stream_base
Ack_queue::PRODUCER))
{ }
using Packet_stream_base::packet_valid;
/**
* Register signal handler to notify that new acknowledgements
* are available in the ack queue.
@ -827,16 +843,6 @@ class Genode::Packet_stream_sink : private Packet_stream_base
*/
bool packet_avail() { return _submit_receiver.ready_for_rx(); }
/**
* Check if packet descriptor refers to a range within the bulk buffer
*/
bool packet_valid(Packet_descriptor packet)
{
return (packet.offset() >= _bulk_buffer_offset
&& packet.offset() < _bulk_buffer_offset + (Genode::off_t)_bulk_buffer_size
&& packet.offset() + packet.size() <= _bulk_buffer_offset + _bulk_buffer_size);
}
/**
* Get next packet from source
*
@ -862,15 +868,11 @@ class Genode::Packet_stream_sink : private Packet_stream_base
/**
* Get pointer to the content of the specified packet
*
* \return 0 if the packet is invalid
* \throw Packet_descriptor::Invalid_packet raise an exception if
the packet is invalid
*/
Content_type *packet_content(Packet_descriptor packet)
{
if (!packet_valid(packet) || packet.size() < sizeof(Content_type))
return 0;
return (Content_type *)((Genode::addr_t)_ds_local_base + packet.offset());
}
Content_type *packet_content(Packet_descriptor packet) {
return Packet_stream_base::packet_content<Content_type>(packet); }
/**
* Returns true if no further acknowledgements can be submitted

View File

@ -143,7 +143,7 @@ class Linux_session_component : public Nic::Session_component
return false;
Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
warning("invalid tx packet");
return true;
}

View File

@ -204,7 +204,7 @@ class Lan9118 : public Nic::Session_component
return false;
Genode::Packet_descriptor packet = _tx.sink()->get_packet();
if (!packet.size()) {
if (!packet.size() || !_tx.sink()->packet_valid(packet)) {
Genode::warning("Invalid tx packet");
return true;
}

View File

@ -62,7 +62,6 @@ class Lx_fs::Session_component : public Session_rpc_object
*/
void _process_packet_op(Packet_descriptor &packet, Open_node &open_node)
{
void * const content = tx_sink()->packet_content(packet);
size_t const length = packet.length();
/* resulting length */
@ -72,8 +71,8 @@ class Lx_fs::Session_component : public Session_rpc_object
switch (packet.operation()) {
case Packet_descriptor::READ:
if (content && (packet.length() <= packet.size())) {
res_length = open_node.node().read((char *)content, length,
if (tx_sink()->packet_valid(packet) && (packet.length() <= packet.size())) {
res_length = open_node.node().read((char *)tx_sink()->packet_content(packet), length,
packet.position());
/* read data or EOF is a success */
@ -82,8 +81,8 @@ class Lx_fs::Session_component : public Session_rpc_object
break;
case Packet_descriptor::WRITE:
if (content && (packet.length() <= packet.size())) {
res_length = open_node.node().write((char const *)content,
if (tx_sink()->packet_valid(packet) && (packet.length() <= packet.size())) {
res_length = open_node.node().write((char const *)tx_sink()->packet_content(packet),
length,
packet.position());

View File

@ -28,7 +28,7 @@ void Packet_handler::_ready_to_submit()
/* as long as packets are available, and we can ack them */
while (sink()->packet_avail()) {
_packet = sink()->get_packet();
if (!_packet.size()) continue;
if (!_packet.size() || !sink()->packet_valid(_packet)) continue;
handle_ethernet(sink()->packet_content(_packet), _packet.size());
if (!sink()->ready_to_ack()) {

View File

@ -70,7 +70,7 @@ void Net::Interface::_ready_to_submit()
while (_sink().packet_avail()) {
Packet_descriptor const pkt = _sink().get_packet();
if (!pkt.size()) {
if (!pkt.size() || !_sink().packet_valid(pkt)) {
continue; }
_handle_eth(_sink().packet_content(pkt), pkt.size(), pkt);

View File

@ -120,8 +120,8 @@ void Nic_loopback::Session_component::_handle_packet_stream()
/* obtain packet */
Packet_descriptor const packet_from_client = _tx.sink()->get_packet();
if (!packet_from_client.size()) {
warning("received zero-size packet");
if (!packet_from_client.size() || !_tx.sink()->packet_valid(packet_from_client)) {
warning("received invalid packet");
_rx.source()->release_packet(packet_to_client);
continue;
}

View File

@ -1379,6 +1379,7 @@ void Interface::_handle_pkt()
_ack_packet(pkt);
}
catch (Packet_postponed) { }
catch (Genode::Packet_descriptor::Invalid_packet) { }
}
@ -1410,6 +1411,11 @@ void Interface::_continue_handle_eth(Domain const &domain,
if (domain.verbose_packet_drop()) {
log("[", domain, "] drop packet (handling postponed twice)"); }
}
catch (Genode::Packet_descriptor::Invalid_packet) {
if (domain.verbose_packet_drop()) {
log("[", domain, "] invalid Nic packet received");
}
}
_ack_packet(pkt);
}

View File

@ -92,7 +92,6 @@ class Block::Session_component : public Block::Session_rpc_object,
bool write = _p_to_handle.operation() == Packet_descriptor::WRITE;
sector_t off = _p_to_handle.block_number() + _partition->lba;
size_t cnt = _p_to_handle.block_count();
void* addr = tx_sink()->packet_content(_p_to_handle);
if (write && !_writeable) {
_ack_packet(_p_to_handle);
@ -100,12 +99,17 @@ class Block::Session_component : public Block::Session_rpc_object,
}
try {
_driver.io(write, off, cnt, addr, *this, _p_to_handle);
_driver.io(write, off, cnt,
tx_sink()->packet_content(_p_to_handle),
*this, _p_to_handle);
} catch (Block::Session::Tx::Source::Packet_alloc_failed) {
if (!_req_queue_full) {
_req_queue_full = true;
Session_component::wait_queue().insert(this);
}
} catch (Genode::Packet_descriptor::Invalid_packet) {
Genode::error("dropping invalid Block packet");
_p_to_handle = Packet_descriptor();
}
}
@ -172,14 +176,19 @@ class Block::Session_component : public Block::Session_rpc_object,
void dispatch(Packet_descriptor &request, Packet_descriptor &reply)
{
request.succeeded(reply.succeeded());
if (request.operation() == Block::Packet_descriptor::READ) {
void *src =
_driver.session().tx()->packet_content(reply);
Genode::size_t sz =
request.block_count() * _driver.blk_size();
Genode::memcpy(tx_sink()->packet_content(request), src, sz);
try { Genode::memcpy(tx_sink()->packet_content(request), src, sz); }
catch (Genode::Packet_descriptor::Invalid_packet) {
request.succeeded(false);
}
}
request.succeeded(reply.succeeded());
_ack_packet(request);
if (_ack_queue_full)

View File

@ -67,7 +67,6 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
*/
void _process_packet_op(Packet_descriptor &packet, Open_node &open_node)
{
void * const content = tx_sink()->packet_content(packet);
size_t const length = packet.length();
/* resulting length */
@ -77,12 +76,12 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
switch (packet.operation()) {
case Packet_descriptor::READ:
if (content && (packet.length() <= packet.size())) {
if (packet.length() <= packet.size()) {
Locked_ptr<Node> node { open_node.node() };
if (!node.valid())
break;
res_length = node->read((char *)content, length,
packet.position());
res_length = node->read((char *)tx_sink()->packet_content(packet),
length, packet.position());
/* read data or EOF is a success */
succeeded = res_length || (packet.position() >= node->status().size);
@ -90,12 +89,12 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
break;
case Packet_descriptor::WRITE:
if (content && (packet.length() <= packet.size())) {
if (packet.length() <= packet.size()) {
Locked_ptr<Node> node { open_node.node() };
if (!node.valid())
break;
res_length = node->write((char const *)content, length,
packet.position());
res_length = node->write((char const *)tx_sink()->packet_content(packet),
length, packet.position());
/* File system session can't handle partial writes */
if (res_length != length) {
@ -150,6 +149,8 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
} catch (Id_space<File_system::Node>::Unknown_id const &) {
Genode::error("Invalid_handle");
tx_sink()->acknowledge_packet(packet);
} catch (Genode::Packet_descriptor::Invalid_packet) {
Genode::error("dropping invalid File_system packet");
}
}

View File

@ -140,13 +140,12 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
*/
void _process_packet_op(Packet_descriptor &packet)
{
void * const content = tx_sink()->packet_content(packet);
size_t const length = packet.length();
seek_off_t const seek = packet.position();
/* assume failure by default */
packet.succeeded(false);
size_t const length = packet.length();
seek_off_t const seek = packet.position();
if ((packet.length() > packet.size()))
return;
@ -166,7 +165,8 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
}
if (node.mode() & READ_ONLY) {
res_length = node.read((char *)content, length, seek);
res_length = node.read(
(char *)tx_sink()->packet_content(packet), length, seek);
/* no way to distinguish EOF from unsuccessful
reads, both have res_length == 0 */
succeeded = true;
@ -184,7 +184,8 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
try {
_apply(packet.handle(), [&] (Io_node &node) {
if (node.mode() & WRITE_ONLY) {
res_length = node.write((char const *)content, length, seek);
res_length = node.write(
(char const *)tx_sink()->packet_content(packet), length, seek);
/* File system session can't handle partial writes */
if (res_length != length) {