From 1fce8d0d7470cf1a91e7a680b395c6c52ca7aa0e Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Mon, 31 Jul 2017 12:53:34 -0500 Subject: [PATCH] default ahci_drv and part_blk Block sessions to read-only Add a "writeable" policy option to the ahci_drv and part_blk Block servers and default from writeable to ready-only. Should a policy permit write acesss the session request argument "writeable" may still downgrade a session to ready-only. Fix #2469 --- .../dde_linux/src/lib/usb/storage/storage.cc | 4 +- repos/dde_rump/src/server/rump_cgd/main.cc | 2 +- repos/gems/src/server/http_blk/main.cc | 2 +- repos/libports/run/libc_filesystem_test.inc | 2 +- repos/os/include/block/component.h | 36 ++++++++++-- repos/os/run/ahci_bench.run | 2 +- repos/os/run/ahci_blk.run | 2 +- repos/os/src/drivers/ahci/README | 11 ++-- repos/os/src/drivers/ahci/main.cc | 56 ++++++++----------- repos/os/src/drivers/sd_card/main.cc | 2 +- repos/os/src/drivers/usb_block/main.cc | 2 +- repos/os/src/server/blk_cache/main.cc | 2 +- repos/os/src/server/part_blk/README | 8 ++- repos/os/src/server/part_blk/component.h | 34 +++++++++-- repos/os/src/server/ram_blk/main.cc | 4 +- repos/os/src/server/rom_blk/main.cc | 4 +- repos/os/src/test/blk/srv/main.cc | 2 +- repos/ports/run/seoul.inc | 2 +- repos/ports/run/virtualbox_auto.inc | 6 +- 19 files changed, 118 insertions(+), 65 deletions(-) diff --git a/repos/dde_linux/src/lib/usb/storage/storage.cc b/repos/dde_linux/src/lib/usb/storage/storage.cc index 711761a73..c1650ac97 100644 --- a/repos/dde_linux/src/lib/usb/storage/storage.cc +++ b/repos/dde_linux/src/lib/usb/storage/storage.cc @@ -222,9 +222,11 @@ void scsi_add_device(struct scsi_device *sdev) * XXX move to 'main' */ if (!announce) { + enum { WRITEABLE = true }; + PREPARE_WORK(&delayed, ack_packet); static Block::Root root(_signal->ep(), Lx::Malloc::mem(), - _signal->rm(), factory); + _signal->rm(), factory, WRITEABLE); _signal->parent().announce(_signal->ep().rpc_ep().manage(&root)); announce = true; } diff --git a/repos/dde_rump/src/server/rump_cgd/main.cc b/repos/dde_rump/src/server/rump_cgd/main.cc index 9079c7f5a..0d41f11bc 100644 --- a/repos/dde_rump/src/server/rump_cgd/main.cc +++ b/repos/dde_rump/src/server/rump_cgd/main.cc @@ -41,7 +41,7 @@ struct Main } factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Main(Genode::Env &env) : env(env) { env.parent().announce(env.ep().manage(root)); } diff --git a/repos/gems/src/server/http_blk/main.cc b/repos/gems/src/server/http_blk/main.cc index 85d068f86..118c59045 100644 --- a/repos/gems/src/server/http_blk/main.cc +++ b/repos/gems/src/server/http_blk/main.cc @@ -102,7 +102,7 @@ struct Main Env &env; Heap heap { env.ram(), env.rm() }; Factory factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Main(Env &env) : env(env) { env.parent().announce(env.ep().manage(root)); } diff --git a/repos/libports/run/libc_filesystem_test.inc b/repos/libports/run/libc_filesystem_test.inc index 35b0e4a71..7a0be23e8 100644 --- a/repos/libports/run/libc_filesystem_test.inc +++ b/repos/libports/run/libc_filesystem_test.inc @@ -105,7 +105,7 @@ append_if $use_ahci config { } append_if $use_ahci config " - " + " append_if $use_ahci config { } diff --git a/repos/os/include/block/component.h b/repos/os/include/block/component.h index fe3c4cccd..3fe7852f5 100644 --- a/repos/os/include/block/component.h +++ b/repos/os/include/block/component.h @@ -76,6 +76,7 @@ class Block::Session_component : public Block::Session_component_base, bool _ack_queue_full; Packet_descriptor _p_to_handle; unsigned _p_in_fly; + bool _writeable; /** * Acknowledge a packet already handled @@ -127,6 +128,10 @@ class Block::Session_component : public Block::Session_component_base, break; case Block::Packet_descriptor::WRITE: + if (!_writeable) { + _ack_packet(_p_to_handle); + break; + } if (_driver.dma_enabled()) _driver.write_dma(packet.block_number(), packet.block_count(), @@ -178,14 +183,16 @@ class Block::Session_component : public Block::Session_component_base, Session_component(Driver_factory &driver_factory, Genode::Entrypoint &ep, Genode::Region_map &rm, - size_t buf_size) + size_t buf_size, + bool writeable) : Session_component_base(driver_factory, buf_size), Driver_session(rm, _rq_ds, ep.rpc_ep()), _rq_phys(Dataspace_client(_rq_ds).phys_addr()), _sink_ack(ep, *this, &Session_component::_signal), _sink_submit(ep, *this, &Session_component::_signal), _req_queue_full(false), - _p_in_fly(0) + _p_in_fly(0), + _writeable(writeable) { _tx.sigh_ready_to_ack(_sink_ack); _tx.sigh_packet_avail(_sink_submit); @@ -232,9 +239,19 @@ class Block::Session_component : public Block::Session_component_base, void info(sector_t *blk_count, size_t *blk_size, Operations *ops) { + Operations driver_ops = _driver.ops(); + *blk_count = _driver.block_count(); *blk_size = _driver.block_size(); - *ops = _driver.ops(); + *ops = Operations(); + + typedef Block::Packet_descriptor::Opcode Opcode; + + if (driver_ops.supported(Opcode::READ)) + ops->set_operation(Opcode::READ); + if (_writeable && driver_ops.supported(Opcode::WRITE)) + ops->set_operation(Opcode::WRITE); + } void sync() { _driver.sync(); } @@ -252,6 +269,7 @@ class Block::Root : public Genode::Root_component - + diff --git a/repos/os/run/ahci_blk.run b/repos/os/run/ahci_blk.run index 33b5e51ca..a993cd7bd 100644 --- a/repos/os/run/ahci_blk.run +++ b/repos/os/run/ahci_blk.run @@ -64,7 +64,7 @@ append config { - + diff --git a/repos/os/src/drivers/ahci/README b/repos/os/src/drivers/ahci/README index 2fef17dfd..064747e54 100644 --- a/repos/os/src/drivers/ahci/README +++ b/repos/os/src/drivers/ahci/README @@ -19,15 +19,18 @@ which client can access a certain device: ! ! ! -! +! ! -! +! +! +! ! ! In the example above, a session request labeled with "test-ahci" gains access to -a device with certain model and serial numbers, while "bench" gains access to -device at port 1. ATAPI support is by default disabled and can be enabled by +a device with certain model and serial numbers, "bench" gains access to +device at port 1, and finally the session "boot_fs" gains read-only access to +port 2. ATAPI support is by default disabled and can be enabled by setting the config attribute "atapi" to "yes". ahci_drv supports reporting of active ports, which can be enabled via diff --git a/repos/os/src/drivers/ahci/main.cc b/repos/os/src/drivers/ahci/main.cc index dc096c701..599557dff 100644 --- a/repos/os/src/drivers/ahci/main.cc +++ b/repos/os/src/drivers/ahci/main.cc @@ -57,8 +57,9 @@ class Session_component : public Block::Session_component Session_component(Block::Driver_factory &driver_factory, Genode::Entrypoint &ep, Genode::Region_map &rm, - Genode::size_t buf_size) - : Block::Session_component(driver_factory, ep, rm, buf_size) { } + Genode::size_t buf_size, + bool writeable) + : Block::Session_component(driver_factory, ep, rm, buf_size, writeable) { } Block::Driver_factory &factory() { return _driver_factory; } }; @@ -73,32 +74,12 @@ class Block::Root_multiple_clients : public Root_component< ::Session_component> Genode::Allocator &_alloc; Genode::Xml_node _config; - long _device_num(Session_label const &label, char *model, char *sn, size_t bufs_len) - { - long num = -1; - - try { - Session_policy policy(label, _config); - - /* try read device port number attribute */ - try { policy.attribute("device").value(&num); } - catch (...) { } - - /* try read device model and serial number attributes */ - model[0] = sn[0] = 0; - policy.attribute("model").value(model, bufs_len); - policy.attribute("serial").value(sn, bufs_len); - - } catch (...) { } - - return num; - } - protected: ::Session_component *_create_session(const char *args) { Session_label const label = label_from_args(args); + Session_policy const policy(label, _config); size_t ram_quota = Arg_string::find_arg(args, "ram_quota").ulong_value(0); @@ -117,18 +98,24 @@ class Block::Root_multiple_clients : public Root_component< ::Session_component> throw Insufficient_ram_quota(); } - /* Search for configured device */ - char model_buf[64], sn_buf[64]; + /* try read device port number attribute */ + long num = policy.attribute_value("device", -1L); + + /* try read device model and serial number attributes */ + auto const model = policy.attribute_value("model", String<64>()); + auto const serial = policy.attribute_value("serial", String<64>()); + + /* sessions are not writeable by default */ + bool writeable = policy.attribute_value("writeable", false); - long num = _device_num(label, model_buf, sn_buf, sizeof(model_buf)); /* prefer model/serial routing */ - if ((model_buf[0] != 0) && (sn_buf[0] != 0)) - num = Ahci_driver::device_number(model_buf, sn_buf); + if ((model != "") && (serial != "")) + num = Ahci_driver::device_number(model.string(), serial.string()); if (num < 0) { error("rejecting session request, no matching policy for '", label, "'", - model_buf[0] == 0 ? "" - : " (model=", Cstring(model_buf), " serial=", Cstring(sn_buf), ")"); + model == "" ? "" + : " (model=", model, " serial=", serial, ")"); throw Service_denied(); } @@ -137,10 +124,15 @@ class Block::Root_multiple_clients : public Root_component< ::Session_component> throw Service_denied(); } + if (writeable) + writeable = Arg_string::find_arg(args, "writeable").bool_value(true); + Block::Factory *factory = new (&_alloc) Block::Factory(num); ::Session_component *session = new (&_alloc) - ::Session_component(*factory, _env.ep(), _env.rm(), tx_buf_size); - log("session opened at device ", num, " for '", label, "'"); + ::Session_component(*factory, _env.ep(), _env.rm(), tx_buf_size, writeable); + log( + writeable ? "writeable " : "read-only ", + "session opened at device ", num, " for '", label, "'"); return session; } diff --git a/repos/os/src/drivers/sd_card/main.cc b/repos/os/src/drivers/sd_card/main.cc index 1bc27d236..f7a921f5a 100644 --- a/repos/os/src/drivers/sd_card/main.cc +++ b/repos/os/src/drivers/sd_card/main.cc @@ -43,7 +43,7 @@ struct Main } factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Main(Genode::Env &env) : env(env) { diff --git a/repos/os/src/drivers/usb_block/main.cc b/repos/os/src/drivers/usb_block/main.cc index 0855f8189..7b51d0c34 100644 --- a/repos/os/src/drivers/usb_block/main.cc +++ b/repos/os/src/drivers/usb_block/main.cc @@ -818,7 +818,7 @@ struct Usb::Main }; Factory factory { env, heap, announce_dispatcher }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Main(Env &env) : env(env) { } }; diff --git a/repos/os/src/server/blk_cache/main.cc b/repos/os/src/server/blk_cache/main.cc index ac43342ee..b3841c9a3 100644 --- a/repos/os/src/server/blk_cache/main.cc +++ b/repos/os/src/server/blk_cache/main.cc @@ -70,7 +70,7 @@ struct Main Genode::Env &env; Genode::Heap heap { env.ram(), env.rm() }; Factory factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Genode::Signal_handler
resource_dispatcher { env.ep(), *this, &Main::resource_handler }; diff --git a/repos/os/src/server/part_blk/README b/repos/os/src/server/part_blk/README index 2dbe595ac..cb7911440 100644 --- a/repos/os/src/server/part_blk/README +++ b/repos/os/src/server/part_blk/README @@ -22,7 +22,7 @@ In order to route a client to the right partition, the server parses its configuration section looking for 'policy' tags. XML Syntax: -! +! part_blk supports partition reporting, which can be enabled via the configuration node. See below for an example. The report @@ -41,6 +41,8 @@ looks like follows (for MBR resp. GPT). ! guid="87199a83-d0f4-4a01-b9e3-6516a8579d61" start="4096" length="16351"/> ! +Clients have read-only access to partitions unless overriden by a 'writeable' +policy attribute. Usage ----- @@ -65,8 +67,8 @@ Configuration snippet with two clients and an (hypothetical) IDE driver: ! 'test-part2' receives access to primary partition 1 --> ! ! -! -! +! +! ! ! ! diff --git a/repos/os/src/server/part_blk/component.h b/repos/os/src/server/part_blk/component.h index 7ae005bb6..ce96ef7fb 100644 --- a/repos/os/src/server/part_blk/component.h +++ b/repos/os/src/server/part_blk/component.h @@ -47,6 +47,7 @@ class Block::Session_component : public Block::Session_rpc_object, Packet_descriptor _p_to_handle; unsigned _p_in_fly; Block::Driver &_driver; + bool _writeable; /** * Acknowledge a packet already handled @@ -84,6 +85,12 @@ class Block::Session_component : public Block::Session_rpc_object, 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); + return; + } + try { _driver.io(write, off, cnt, addr, *this, _p_to_handle); } catch (Block::Session::Tx::Source::Packet_alloc_failed) { @@ -126,7 +133,8 @@ class Block::Session_component : public Block::Session_rpc_object, Partition *partition, Genode::Entrypoint &ep, Genode::Region_map &rm, - Block::Driver &driver) + Block::Driver &driver, + bool writeable) : Session_rpc_object(rm, rq_ds, ep.rpc_ep()), _rq_ds(rq_ds), _rq_phys(Dataspace_client(_rq_ds).phys_addr()), @@ -136,7 +144,8 @@ class Block::Session_component : public Block::Session_rpc_object, _req_queue_full(false), _ack_queue_full(false), _p_in_fly(0), - _driver(driver) + _driver(driver), + _writeable(writeable) { _tx.sigh_ready_to_ack(_sink_ack); _tx.sigh_packet_avail(_sink_submit); @@ -193,9 +202,18 @@ class Block::Session_component : public Block::Session_rpc_object, void info(sector_t *blk_count, size_t *blk_size, Operations *ops) { + Operations driver_ops = _driver.ops(); + *blk_count = _partition->sectors; *blk_size = _driver.blk_size(); - *ops = _driver.ops(); + *ops = Operations(); + + typedef Block::Packet_descriptor::Opcode Opcode; + + if (driver_ops.supported(Opcode::READ)) + ops->set_operation(Opcode::READ); + if (_writeable && driver_ops.supported(Opcode::WRITE)) + ops->set_operation(Opcode::WRITE); } void sync() { _driver.session().sync(); } @@ -230,6 +248,7 @@ class Block::Root : Session_component *_create_session(const char *args) override { long num = -1; + bool writeable = false; Session_label const label = label_from_args(args); char const *label_str = label.string(); @@ -239,6 +258,9 @@ class Block::Root : /* read partition attribute */ policy.attribute("partition").value(&num); + /* sessions are not writeable by default */ + writeable = policy.attribute_value("writeable", false); + } catch (Xml_node::Nonexistent_attribute) { error("policy does not define partition number for for '", label_str, "'"); @@ -280,11 +302,15 @@ class Block::Root : throw Insufficient_ram_quota(); } + if (writeable) + writeable = Arg_string::find_arg(args, "writeable").bool_value(true); + Ram_dataspace_capability ds_cap; ds_cap = _env.ram().alloc(tx_buf_size); Session_component *session = new (md_alloc()) Session_component(ds_cap, _table.partition(num), - _env.ep(), _env.rm(), _driver); + _env.ep(), _env.rm(), _driver, + writeable); log("session opened at partition ", num, " for '", label_str, "'"); return session; diff --git a/repos/os/src/server/ram_blk/main.cc b/repos/os/src/server/ram_blk/main.cc index 5ef56308f..9f6268b80 100644 --- a/repos/os/src/server/ram_blk/main.cc +++ b/repos/os/src/server/ram_blk/main.cc @@ -194,7 +194,9 @@ struct Main Genode::destroy(&alloc, driver); } } factory { env, heap, config_rom.xml() }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + enum { WRITEABLE = true }; + + Block::Root root { env.ep(), heap, env.rm(), factory, WRITEABLE }; Main(Env &env) : env(env) { diff --git a/repos/os/src/server/rom_blk/main.cc b/repos/os/src/server/rom_blk/main.cc index d6e9802aa..3fc60090a 100644 --- a/repos/os/src/server/rom_blk/main.cc +++ b/repos/os/src/server/rom_blk/main.cc @@ -117,7 +117,9 @@ struct Main Genode::destroy(&heap, driver); } } factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + enum { WRITEABLE = false }; + + Block::Root root { env.ep(), heap, env.rm(), factory, WRITEABLE }; Main(Env &env) : env(env) { env.parent().announce(env.ep().manage(root)); } diff --git a/repos/os/src/test/blk/srv/main.cc b/repos/os/src/test/blk/srv/main.cc index 31071cd0c..c5df4633f 100644 --- a/repos/os/src/test/blk/srv/main.cc +++ b/repos/os/src/test/blk/srv/main.cc @@ -124,7 +124,7 @@ struct Main void destroy(Block::Driver *driver) { } } factory { env, heap }; - Block::Root root { env.ep(), heap, env.rm(), factory }; + Block::Root root { env.ep(), heap, env.rm(), factory, true }; Timer::Connection timer { env }; Genode::Signal_handler dispatcher { env.ep(), *factory.driver, &Driver::handler }; diff --git a/repos/ports/run/seoul.inc b/repos/ports/run/seoul.inc index 408760bc8..75a215538 100644 --- a/repos/ports/run/seoul.inc +++ b/repos/ports/run/seoul.inc @@ -181,7 +181,7 @@ append_if $use_block_sata config { - + } diff --git a/repos/ports/run/virtualbox_auto.inc b/repos/ports/run/virtualbox_auto.inc index a093360fe..cfa361e01 100644 --- a/repos/ports/run/virtualbox_auto.inc +++ b/repos/ports/run/virtualbox_auto.inc @@ -92,7 +92,7 @@ append config { - + @@ -103,9 +103,9 @@ append config { } append_if [expr $use_rumpfs] config { - } + } append_if [expr !$use_rumpfs] config { - } + } append config { }