block: prevent from dereferencing invalid pointers

Until now, block drivers had to deal with a pointer to the client
session component, e.g.: to acknowledge block packets already processed.
When a session was closed, the driver object wasn't informed explicitly,
which leads to defensive programming, or lastly to a race-condition in
test-blk-srv. To prevent from this class of errors, the pointer is now
private to the generic block driver base class, and not accessible to
the concrete driver implementation. Moreover, the driver gets explicitly
informed when a session got invalidated.

Ref #113
This commit is contained in:
Stefan Kalkowski 2014-02-06 15:23:17 +01:00 committed by Norman Feske
parent ca513113f6
commit eeb2d95b1f
14 changed files with 262 additions and 185 deletions

View File

@ -37,18 +37,7 @@ class Storage_device : public Genode::List<Storage_device>::Element,
static void _sync_done(struct scsi_cmnd *cmnd) {
complete((struct completion *)cmnd->back); }
static void _async_done(struct scsi_cmnd *cmnd)
{
Block::Session_component *session = static_cast<Block::Session_component *>(cmnd->session);
Block::Packet_descriptor *packet = static_cast<Block::Packet_descriptor *>(cmnd->packet);
if (verbose)
PDBG("ACK packet for block: %llu status: %d", packet->block_number(), cmnd->result);
session->ack_packet(*packet);
Genode::destroy(Genode::env()->heap(), packet);
_scsi_free_command(cmnd);
}
static void _async_done(struct scsi_cmnd *cmnd);
void _capacity()
{
@ -88,7 +77,7 @@ class Storage_device : public Genode::List<Storage_device>::Element,
void _io(Block::sector_t block_nr, Genode::size_t block_count,
Block::Packet_descriptor packet,
Genode::addr_t virt, Genode::addr_t phys, bool read)
Genode::addr_t phys, bool read)
{
if (block_nr > _block_count)
throw Io_error();
@ -108,7 +97,6 @@ class Storage_device : public Genode::List<Storage_device>::Element,
Block::Packet_descriptor *p = new (Genode::env()->heap()) Block::Packet_descriptor();
*p = packet;
cmnd->packet = (void *)p;
cmnd->session = (void *)session;
Genode::uint32_t be_block_nr = bswap<Genode::uint32_t>(block_nr);
memcpy(&cmnd->cmnd[2], &be_block_nr, 4);
@ -118,7 +106,7 @@ class Storage_device : public Genode::List<Storage_device>::Element,
memcpy(&cmnd->cmnd[7], &be_block_count, 2);
/* setup command */
scsi_setup_buffer(cmnd, block_count * _block_size, (void *)virt, phys);
scsi_setup_buffer(cmnd, block_count * _block_size, (void *)0, phys);
/*
* Required by 'last_sector_hacks' in 'drivers/usb/storage/transprot.c
@ -156,22 +144,14 @@ class Storage_device : public Genode::List<Storage_device>::Element,
void read_dma(Block::sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Block::Packet_descriptor &packet)
{
_io(block_number, block_count, packet,
(Genode::addr_t)session->tx_sink()->packet_content(packet),
phys, true);
}
Block::Packet_descriptor &packet) {
_io(block_number, block_count, packet, phys, true); }
void write_dma(Block::sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Block::Packet_descriptor &packet)
{
_io(block_number, block_count, packet,
(Genode::addr_t)session->tx_sink()->packet_content(packet),
phys, false);
}
Block::Packet_descriptor &packet) {
_io(block_number, block_count, packet, phys, false); }
bool dma_enabled() { return true; }
@ -198,12 +178,16 @@ struct Factory : Block::Driver_factory
};
static Storage_device *device = nullptr;
void scsi_add_device(struct scsi_device *sdev)
{
using namespace Genode;
static bool announce = false;
static struct Factory factory(sdev);
device = &factory.device;
/*
* XXX move to 'main'
@ -215,3 +199,17 @@ void scsi_add_device(struct scsi_device *sdev)
}
}
void Storage_device::_async_done(struct scsi_cmnd *cmnd)
{
Block::Packet_descriptor *packet =
static_cast<Block::Packet_descriptor *>(cmnd->packet);
if (verbose)
PDBG("ACK packet for block: %llu status: %d",
packet->block_number(), cmnd->result);
device->ack_packet(*packet);
Genode::destroy(Genode::env()->heap(), packet);
_scsi_free_command(cmnd);
}

View File

@ -56,7 +56,7 @@ class Driver : public Block::Driver
{
_http.cmd_get(block_nr * _block_size, block_count * _block_size,
(addr_t)buffer);
session->ack_packet(packet);
ack_packet(packet);
}
};

View File

@ -18,7 +18,6 @@
#include <root/component.h>
#include <os/signal_rpc_dispatcher.h>
#include <os/server.h>
#include <block_session/rpc_object.h>
#include <block/driver.h>
namespace Block {
@ -64,7 +63,7 @@ class Block::Session_component_base
class Block::Session_component : public Block::Session_component_base,
public Block::Session_rpc_object
public Block::Driver_session
{
private:
@ -183,7 +182,7 @@ class Block::Session_component : public Block::Session_component_base,
Session_component(Driver_factory &driver_factory,
Server::Entrypoint &ep, size_t buf_size)
: Session_component_base(driver_factory, buf_size),
Session_rpc_object(_rq_ds, ep.rpc_ep()),
Driver_session(_rq_ds, ep.rpc_ep()),
_rq_phys(Dataspace_client(_rq_ds).phys_addr()),
_sink_ack(ep, *this, &Session_component::_ready_to_ack),
_sink_submit(ep, *this, &Session_component::_packet_avail),
@ -193,9 +192,11 @@ class Block::Session_component : public Block::Session_component_base,
_tx.sigh_ready_to_ack(_sink_ack);
_tx.sigh_packet_avail(_sink_submit);
_driver.session = this;
_driver.session(this);
}
~Session_component() { _driver.session(nullptr); }
/**
* Acknowledges a packet processed by the driver to the client
*
@ -204,7 +205,7 @@ class Block::Session_component : public Block::Session_component_base,
*
* \throw Ack_congestion
*/
void ack_packet(Packet_descriptor &packet, bool success = true)
void ack_packet(Packet_descriptor &packet, bool success)
{
packet.succeeded(success);
_ack_packet(packet);

View File

@ -21,145 +21,204 @@
#include <base/signal.h>
#include <ram_session/ram_session.h>
#include <block_session/block_session.h>
#include <block_session/rpc_object.h>
namespace Block {
class Session_component;
struct Driver;
class Driver_session;
class Driver;
struct Driver_factory;
};
class Block::Driver_session : public Block::Session_rpc_object
{
public:
/**
* Constructor
*
* \param tx_ds dataspace used as communication buffer
* for the tx packet stream
* \param ep entry point used for packet-stream channel
*/
Driver_session(Genode::Dataspace_capability tx_ds,
Genode::Rpc_entrypoint &ep)
: Session_rpc_object(tx_ds, ep) { }
/**
* Acknowledges a packet processed by the driver to the client
*
* \param packet the packet to acknowledge
* \param success indicated whether the processing was successful
*
* \throw Ack_congestion
*/
virtual void ack_packet(Packet_descriptor &packet,
bool success) = 0;
};
/**
* Interface to be implemented by the device-specific driver code
*/
struct Block::Driver
class Block::Driver
{
Session_component * session; /* single session component of the driver
* might get used to acknowledge requests */
private:
/**
* Exceptions
*/
class Io_error : public ::Genode::Exception { };
class Request_congestion : public ::Genode::Exception { };
Driver_session *_session = nullptr;
/**
* Request block size for driver and medium
*/
virtual Genode::size_t block_size() = 0;
public:
/**
* Request capacity of medium in blocks
*/
virtual Block::sector_t block_count() = 0;
/**
* Exceptions
*/
class Io_error : public ::Genode::Exception { };
class Request_congestion : public ::Genode::Exception { };
/**
* Request operations supported by the device
*/
virtual Session::Operations ops() = 0;
/**
* Request block size for driver and medium
*/
virtual Genode::size_t block_size() = 0;
/**
* Read from medium
*
* \param block_number number of first block to read
* \param block_count number of blocks to read
* \param buffer output buffer for read request
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA non-capable devices
*/
virtual void read(sector_t block_number,
Genode::size_t block_count,
char * buffer,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Request capacity of medium in blocks
*/
virtual Block::sector_t block_count() = 0;
/**
* Write to medium
*
* \param block_number number of first block to write
* \param block_count number of blocks to write
* \param buffer buffer for write request
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA non-capable, non-ROM devices
*/
virtual void write(sector_t block_number,
Genode::size_t block_count,
const char * buffer,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Request operations supported by the device
*/
virtual Session::Operations ops() = 0;
/**
* Read from medium using DMA
*
* \param block_number number of first block to read
* \param block_count number of blocks to read
* \param phys phyiscal address of read buffer
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA capable devices
*/
virtual void read_dma(sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Read from medium
*
* \param block_number number of first block to read
* \param block_count number of blocks to read
* \param buffer output buffer for read request
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA non-capable devices
*/
virtual void read(sector_t block_number,
Genode::size_t block_count,
char * buffer,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Write to medium using DMA
*
* \param block_number number of first block to write
* \param block_count number of blocks to write
* \param phys physical address of write buffer
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA capable, non-ROM devices
*/
virtual void write_dma(sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Write to medium
*
* \param block_number number of first block to write
* \param block_count number of blocks to write
* \param buffer buffer for write request
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA non-capable, non-ROM devices
*/
virtual void write(sector_t block_number,
Genode::size_t block_count,
const char * buffer,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Check if DMA is enabled for driver
*
* \return true if DMA is enabled, false otherwise
*
* Note: has to be overriden by DMA-capable devices
*/
virtual bool dma_enabled() { return false; }
/**
* Read from medium using DMA
*
* \param block_number number of first block to read
* \param block_count number of blocks to read
* \param phys phyiscal address of read buffer
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA capable devices
*/
virtual void read_dma(sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Allocate buffer which is suitable for DMA.
*/
virtual Genode::Ram_dataspace_capability
alloc_dma_buffer(Genode::size_t size) {
return Genode::env()->ram_session()->alloc(size, false); }
/**
* Write to medium using DMA
*
* \param block_number number of first block to write
* \param block_count number of blocks to write
* \param phys physical address of write buffer
* \param packet packet descriptor from the client
*
* \throw Request_congestion
*
* Note: should be overridden by DMA capable, non-ROM devices
*/
virtual void write_dma(sector_t block_number,
Genode::size_t block_count,
Genode::addr_t phys,
Packet_descriptor &packet) {
throw Io_error(); }
/**
* Free buffer which is suitable for DMA.
*/
virtual void free_dma_buffer(Genode::Ram_dataspace_capability c) {
return Genode::env()->ram_session()->free(c); }
/**
* Check if DMA is enabled for driver
*
* \return true if DMA is enabled, false otherwise
*
* Note: has to be overriden by DMA-capable devices
*/
virtual bool dma_enabled() { return false; }
/**
* Synchronize with device.
*
* Note: should be overriden by (e.g. intermediate) components,
* which cache data
*/
virtual void sync() {}
/**
* Allocate buffer which is suitable for DMA.
*
* Note: has to be overriden by DMA-capable devices
*/
virtual Genode::Ram_dataspace_capability
alloc_dma_buffer(Genode::size_t size) {
return Genode::env()->ram_session()->alloc(size); }
/**
* Free buffer which is suitable for DMA.
*
* Note: has to be overriden by DMA-capable devices
*/
virtual void free_dma_buffer(Genode::Ram_dataspace_capability c) {
return Genode::env()->ram_session()->free(c); }
/**
* Synchronize with device.
*
* Note: should be overriden by (e.g. intermediate) components,
* which cache data
*/
virtual void sync() {}
/**
* Informs the driver that the client session was closed
*
* Note: drivers with state (e.g. asynchronously working)
* should override this function, and reset their internal state
*/
virtual void session_invalidated() { }
/**
* Set single session component of the driver
*
* Session might get used to acknowledge requests.
*/
void session(Driver_session *session) {
if (!(_session = session)) session_invalidated(); }
/**
* Acknowledge a packet after processing finished to the client
*
* \param p packet to acknowledge
*/
void ack_packet(Packet_descriptor &p, bool success = true) {
if (_session) _session->ack_packet(p, success); }
};

View File

@ -56,12 +56,18 @@ class Ahci_driver : public Block::Driver
Block::sector_t block_count();
bool dma_enabled() { return true; }
Genode::Ram_dataspace_capability alloc_dma_buffer(Genode::size_t size) {
return Genode::env()->ram_session()->alloc(size, false); }
void free_dma_buffer(Genode::Ram_dataspace_capability c) {
return Genode::env()->ram_session()->free(c); }
void read_dma(Block::sector_t block_nr, size_t block_cnt, addr_t phys,
Block::Packet_descriptor &packet)
{
if (_ncq_command(block_nr, block_cnt, phys, 0))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
void write_dma(Block::sector_t block_nr, size_t block_cnt, addr_t phys,
@ -69,7 +75,7 @@ class Ahci_driver : public Block::Driver
{
if (_ncq_command(block_nr, block_cnt, phys, 1))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
};

View File

@ -66,7 +66,7 @@ class Ahci_driver_base : public Block::Driver
{
_sanity_check(block_number, block_count);
_device->read(block_number, block_count, phys);
if (session) session->ack_packet(packet);
ack_packet(packet);
}
void write_dma(Block::sector_t block_number,
@ -76,7 +76,7 @@ class Ahci_driver_base : public Block::Driver
{
_sanity_check(block_number, block_count);
_device->write(block_number, block_count, phys);
if (session) session->ack_packet(packet);
ack_packet(packet);
}
Ram_dataspace_capability alloc_dma_buffer(size_t size) {

View File

@ -124,7 +124,7 @@ namespace Ata {
Block::Packet_descriptor &packet)
{
_read(block_number, block_count, buffer, false);
session->ack_packet(packet);
ack_packet(packet);
}
void write(Block::sector_t block_number,
@ -133,7 +133,7 @@ namespace Ata {
Block::Packet_descriptor &packet)
{
_write(block_number, block_count, buffer, false);
session->ack_packet(packet);
ack_packet(packet);
}
void read_dma(Block::sector_t block_number,
@ -142,7 +142,7 @@ namespace Ata {
Block::Packet_descriptor &packet)
{
_read(block_number, block_count, (char*)phys, true);
session->ack_packet(packet);
ack_packet(packet);
}
void write_dma(Block::sector_t block_number,
@ -151,7 +151,7 @@ namespace Ata {
Block::Packet_descriptor &packet)
{
_write(block_number, block_count, (char*)phys, true);
session->ack_packet(packet);
ack_packet(packet);
}
bool dma_enabled() { return _dma; }

View File

@ -104,7 +104,7 @@ class Block::Exynos5_driver : public Block::Driver
{
if (!_controller.read_blocks_dma(block_number, block_count, phys))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
void write_dma(Block::sector_t block_number,
@ -114,7 +114,7 @@ class Block::Exynos5_driver : public Block::Driver
{
if (!_controller.write_blocks_dma(block_number, block_count, phys))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
bool dma_enabled() { return _use_dma; }

View File

@ -97,7 +97,7 @@ class Block::Omap4_driver : public Block::Driver
{
if (!_controller.read_blocks(block_number, block_count, out_buffer))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
void write(Block::sector_t block_number,
@ -107,7 +107,7 @@ class Block::Omap4_driver : public Block::Driver
{
if (!_controller.write_blocks(block_number, block_count, buffer))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
void read_dma(Block::sector_t block_number,
@ -117,7 +117,7 @@ class Block::Omap4_driver : public Block::Driver
{
if (!_controller.read_blocks_dma(block_number, block_count, phys))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
void write_dma(Block::sector_t block_number,
@ -127,10 +127,16 @@ class Block::Omap4_driver : public Block::Driver
{
if (!_controller.write_blocks_dma(block_number, block_count, phys))
throw Io_error();
session->ack_packet(packet);
ack_packet(packet);
}
bool dma_enabled() { return _use_dma; }
Genode::Ram_dataspace_capability alloc_dma_buffer(Genode::size_t size) {
return Genode::env()->ram_session()->alloc(size, false); }
void free_dma_buffer(Genode::Ram_dataspace_capability c) {
return Genode::env()->ram_session()->free(c); }
};
#endif /* _DRIVER_H_ */

View File

@ -107,7 +107,7 @@ class Sd_card : public Block::Driver
length, &resp);
_hd.read_data(length, out_buffer + (i * BLOCK_SIZE));
}
session->ack_packet(packet);
ack_packet(packet);
}
void write(Block::sector_t block_number,
@ -129,7 +129,7 @@ class Sd_card : public Block::Driver
length, &resp);
_hd.write_data(length, buffer + (i * BLOCK_SIZE));
}
session->ack_packet(packet);
ack_packet(packet);
}
};

View File

@ -35,10 +35,12 @@ class Driver : public Block::Driver
{
Block::Packet_descriptor srv;
Block::Packet_descriptor cli;
char * const buffer;
Request(Block::Packet_descriptor &s,
Block::Packet_descriptor &c)
: srv(s), cli(c) {}
Block::Packet_descriptor &c,
char * const b)
: srv(s), cli(c), buffer(b) {}
/*
* \return true when the given response packet matches
@ -158,12 +160,10 @@ class Driver : public Block::Driver
try {
if (r->cli.operation() == Block::Packet_descriptor::READ)
read(r->cli.block_number(), r->cli.block_count(),
session->tx_sink()->packet_content(r->cli),
r->cli);
r->buffer, r->cli);
else
write(r->cli.block_number(), r->cli.block_count(),
session->tx_sink()->packet_content(r->cli),
r->cli);
r->buffer, r->cli);
} catch(Block::Driver::Request_congestion) {
PWRN("cli (%lld %zu) srv (%lld %zu)",
r->cli.block_number(), r->cli.block_count(),
@ -214,6 +214,7 @@ class Driver : public Block::Driver
*/
void _request(Block::sector_t block_number,
Genode::size_t block_count,
char * const buffer,
Block::Packet_descriptor &packet)
{
Block::Packet_descriptor p_to_dev;
@ -222,7 +223,8 @@ class Driver : public Block::Driver
/* we've to look whether the request is already pending */
for (Request *r = _r_list.first(); r; r = r->next()) {
if (r->match(false, block_number, block_count)) {
_r_list.insert(new (&_r_slab) Request(r->srv, packet));
_r_list.insert(new (&_r_slab) Request(r->srv, packet,
buffer));
return;
}
}
@ -246,7 +248,7 @@ class Driver : public Block::Driver
Block::Packet_descriptor(_blk.dma_alloc_packet(_blk_sz*cnt),
Block::Packet_descriptor::READ,
nr, cnt);
_r_list.insert(new (&_r_slab) Request(p_to_dev, packet));
_r_list.insert(new (&_r_slab) Request(p_to_dev, packet, buffer));
_blk.tx()->submit_packet(p_to_dev);
} catch(Block::Session::Tx::Source::Packet_alloc_failed) {
throw Request_congestion();
@ -290,7 +292,7 @@ class Driver : public Block::Driver
* \param p client side packet, which triggered this operation
*/
bool _stat(Block::sector_t nr, Genode::size_t cnt,
Block::Packet_descriptor &p)
char * const buffer, Block::Packet_descriptor &p)
{
Cache::offset_t off = nr * _blk_sz;
Cache::size_t size = cnt * _blk_sz;
@ -302,7 +304,7 @@ class Driver : public Block::Driver
} catch(Cache::Chunk_base::Range_incomplete &e) {
off = Genode::max(off, e.off);
size = Genode::min(end - off, e.size);
_request(off / _blk_sz, size / _blk_sz, p);
_request(off / _blk_sz, size / _blk_sz, buffer, p);
}
return false;
}
@ -396,11 +398,11 @@ class Driver : public Block::Driver
if (!_ops.supported(Block::Packet_descriptor::READ))
throw Io_error();
if (!_stat(block_number, block_count, packet))
if (!_stat(block_number, block_count, buffer, packet))
return;
_cache.read(buffer, block_count*_blk_sz, block_number*_blk_sz);
session->ack_packet(packet);
ack_packet(packet);
}
void write(Block::sector_t block_number,
@ -413,16 +415,18 @@ class Driver : public Block::Driver
_cache.alloc(block_count * _blk_sz, block_number * _blk_sz);
if ((block_number % _cache_blk_mod()) && _stat(block_number, 1, packet))
if ((block_number % _cache_blk_mod()) &&
_stat(block_number, 1, const_cast<char* const>(buffer), packet))
return;
if (((block_number+block_count) % _cache_blk_mod())
&& _stat(block_number+block_count-1, 1, packet))
&& _stat(block_number+block_count-1, 1,
const_cast<char* const>(buffer), packet))
return;
_cache.write(buffer, block_count * _blk_sz,
block_number * _blk_sz);
session->ack_packet(packet);
ack_packet(packet);
}
void sync() { _sync(); }

View File

@ -76,7 +76,7 @@ class Rom_blk : public Block::Driver
/* copy file content to packet payload */
memcpy((void*)buffer, (void*)(_file_addr + offset), size);
session->ack_packet(packet);
ack_packet(packet);
}
};

View File

@ -46,7 +46,7 @@ class Driver : public Block::Driver
{
while (!_packets.empty()) {
Block::Packet_descriptor p = _packets.get();
session->ack_packet(p);
ack_packet(p);
}
}
@ -55,6 +55,9 @@ class Driver : public Block::Driver
** Block::Driver interface **
*******************************/
void session_invalidated() {
while (!_packets.empty()) _packets.get(); }
Genode::size_t block_size() { return _size; }
Block::sector_t block_count() { return _number; }

View File

@ -72,7 +72,7 @@ class Driver : public Block::Driver
Genode::size_t size = block_count * BLOCK_SIZE;
Genode::memcpy((void*)buffer, (void*)(_fb_addr + offset), size);
session->ack_packet(packet);
ack_packet(packet);
}
void write(Block::sector_t block_number,
@ -92,7 +92,7 @@ class Driver : public Block::Driver
Genode::memcpy((void*)(_fb_addr + offset), (void*)buffer, size);
_fb.refresh(0, 0, _fb_mode.width(), _fb_mode.height());
session->ack_packet(packet);
ack_packet(packet);
}
};