Noux: move ELF signature check into 'Child_env'

Fixes #2703

repos/ports/include/noux_session/sysio.h

@@ -319,9 +319,10 @@ struct Noux::Sysio
 	                       SYMLINK_ERR_NO_SPACE, SYMLINK_ERR_NO_PERM,
 	                       SYMLINK_ERR_NAME_TOO_LONG };
 
-	enum Execve_error    { EXECVE_NONEXISTENT = Vfs::Directory_service::NUM_GENERAL_ERRORS,
+	enum Execve_error    { EXECVE_ERR_NO_ENTRY = Vfs::Directory_service::NUM_GENERAL_ERRORS,
-	                       EXECVE_NOMEM,
+	                       EXECVE_ERR_NO_MEMORY,
-	                       EXECVE_NOEXEC };
+	                       EXECVE_ERR_NO_EXEC,
+	                       EXECVE_ERR_ACCESS};
 	enum Fork_error      { FORK_NOMEM = Vfs::Directory_service::NUM_GENERAL_ERRORS };
 	enum Select_error    { SELECT_ERR_INTERRUPT };
 

repos/ports/src/lib/libc_noux/plugin.cc

@@ -1099,9 +1099,10 @@ namespace {
 		if (!noux_syscall(Noux::Session::SYSCALL_EXECVE)) {
 			warning("exec syscall failed for path \"", filename, "\"");
 			switch (sysio()->error.execve) {
-			case Noux::Sysio::EXECVE_NONEXISTENT: errno = ENOENT; break;
+			case Noux::Sysio::EXECVE_ERR_NO_ENTRY:  errno = ENOENT; break;
-			case Noux::Sysio::EXECVE_NOMEM:       errno = ENOMEM; break;
+			case Noux::Sysio::EXECVE_ERR_NO_MEMORY: errno = ENOMEM; break;
-			case Noux::Sysio::EXECVE_NOEXEC:      errno = ENOEXEC; break;
+			case Noux::Sysio::EXECVE_ERR_NO_EXEC:   errno = ENOEXEC; break;
+			case Noux::Sysio::EXECVE_ERR_ACCESS:    errno = EACCES; break;
 			}
 			return -1;
 		}

repos/ports/src/noux/child.h

@@ -300,7 +300,6 @@ class Noux::Child : public Rpc_object<Session>,
 
 	public:
 
-		struct Binary_does_not_exist : Exception { };
 		struct Insufficient_memory : Exception { };
 
 		/**
@@ -311,10 +310,6 @@ class Noux::Child : public Rpc_object<Session>,
 		 *                or children created via execve, or
 		 *                true if the child is a fork from another child
 		 *
-		 * \throw Binary_does_not_exist  if child is not a fork and the
-		 *                               specified name could not be
-		 *                               looked up at the virtual file
-		 *                               system
 		 * \throw Insufficent_memory if the child could not be started by
 		 *                           the parent
 		 */

repos/ports/src/noux/child_env.h

@@ -48,38 +48,79 @@ class Noux::Child_env
 			memcpy(_env, env, sizeof(Sysio::Env));
 		}
 
+		/**
+		 * Verify that the file exists and return its size
+		 */
+		Vfs::file_size _file_size(Vfs::Dir_file_system &root_dir,
+		                          char const *binary_name)
+		{
+			Vfs::Directory_service::Stat stat_out;
+			Vfs::Directory_service::Stat_result stat_result;
+			
+			stat_result = root_dir.stat(binary_name, stat_out);
+
+			switch (stat_result) {
+			case Vfs::Directory_service::STAT_OK:
+				break;
+			case Vfs::Directory_service::STAT_ERR_NO_ENTRY:
+				throw Binary_does_not_exist();
+			case Vfs::Directory_service::STAT_ERR_NO_PERM:
+				throw Binary_is_not_accessible();
+			}
+
+			return stat_out.size;
+		}
+
+		/**
+		 * Verify that the file is a valid ELF executable
+		 */
+		void _verify_elf(char const *file)
+		{
+			if ((file[0] != 0x7f) ||
+			    (file[1] != 'E') ||
+			    (file[2] != 'L') ||
+			    (file[3] != 'F'))
+			    throw Binary_is_not_executable();
+		}
+
 		/**
 		 * Handle the case that the given binary needs an interpreter
 		 */
-		void _process_binary_name_and_args(Region_map &local_rm,
+		void _process_binary_name_and_args(char const *binary_name,
-		                                   const char *binary_name,
+		                                   char const *args,
-		                                   Dataspace_capability binary_ds,
+		                                   Vfs::Dir_file_system &root_dir,
-		                                   const char *args)
+		                                   Vfs_io_waiter_registry &vfs_io_waiter_registry,
+		                                   Ram_session &ram,
+		                                   Region_map &rm,
+		                                   Allocator &alloc)
 		{
-			bool interpretable = true;
+			Vfs::file_size binary_size = _file_size(root_dir, binary_name);
 
-			const size_t binary_size = Dataspace_client(binary_ds).size();
+			if (binary_size == 0)
+				throw Binary_is_not_executable();
 
-			if (binary_size < 4)
+			/*
-				interpretable = false;
+			 * We may have to check the dataspace twice because the binary
+			 * could be a script that uses an interpreter which might not
+			 * exist.
+			 */
+			Reconstructible<Vfs_dataspace> binary_ds {
+				root_dir, vfs_io_waiter_registry,
+				binary_name, ram, rm, alloc
+			};
 
-			const char *binary_addr = 0;
+			if (!binary_ds->ds.valid())
-			if (interpretable)
+				throw Binary_is_not_executable();
-				try {
-					binary_addr = local_rm.attach(binary_ds);
-				} catch(...)  {
-					warning("could not attach dataspace");
-					interpretable = false;
-				}
 
-			if (interpretable &&
+			Reconstructible<Attached_dataspace> attached_binary_ds(rm, binary_ds->ds);
-			    ((binary_addr[0] != '#') || (binary_addr[1] != '!')))
-				interpretable = false;
 
-			if (!interpretable) {
+			char const *binary_addr = attached_binary_ds->local_addr<char const>();
-				local_rm.detach(binary_addr);
+
+			/* look for '#!' */
+			if ((binary_addr[0] != '#') || (binary_addr[1] != '!')) {
 				_binary_name = binary_name;
 				Genode::memcpy(_args, args, ARGS_SIZE);
+				_verify_elf(binary_addr);
 				return;
 			}
 
@@ -102,7 +143,7 @@ class Noux::Child_env
 
 			/* no interpreter name found */
 			if (interpreter_line_cursor == eol)
-				throw Child::Binary_does_not_exist();
+				throw Binary_does_not_exist();
 
 			int interpreter_name_start = interpreter_line_cursor;
 
@@ -142,16 +183,43 @@ class Noux::Child_env
 			Genode::memcpy(&_args[args_buf_cursor],
 			               args, ARGS_SIZE);
 
-			local_rm.detach(binary_addr);
+			/* check if interpreter exists and is executable */
+
+			binary_size = _file_size(root_dir, _binary_name);
+
+			if (binary_size == 0)
+				throw Binary_is_not_executable();
+
+			binary_ds.construct(root_dir, vfs_io_waiter_registry,
+			                    _binary_name, ram,
+			                    rm, alloc);
+
+			if (!binary_ds->ds.valid())
+				throw Binary_is_not_executable();
+
+			attached_binary_ds.construct(rm, binary_ds->ds);
+
+			_verify_elf(attached_binary_ds->local_addr<char const>());
 		}
 
 	public:
 
-		Child_env(Region_map &local_rm, const char *binary_name,
+		struct Binary_does_not_exist    : Exception { };
-		          Dataspace_capability binary_ds, const char *args, Sysio::Env env)
+		struct Binary_is_not_accessible : Exception { };
+		struct Binary_is_not_executable : Exception { };
+
+		Child_env(char const *binary_name,
+		          char const *args, Sysio::Env env,
+		          Vfs::Dir_file_system &root_dir,
+		          Vfs_io_waiter_registry &vfs_io_waiter_registry,
+		          Ram_session &ram,
+		          Region_map &rm,
+		          Allocator &alloc)
 		{
 			_process_env(env);
-			_process_binary_name_and_args(local_rm, binary_name, binary_ds, args);
+			_process_binary_name_and_args(binary_name, args, root_dir,
+			                              vfs_io_waiter_registry,
+			                              ram, rm, alloc);
 		}
 
 		char const *binary_name() const { return _binary_name; }

repos/ports/src/noux/rom_session_component.h

@@ -63,9 +63,13 @@ struct Noux::Vfs_dataspace
 			got_ds_from_vfs = false;
 
 			Vfs::Directory_service::Stat stat_out;
+
 			if (root_dir.stat(name.string(), stat_out) != Vfs::Directory_service::STAT_OK)
 				return;
 
+			if (stat_out.size == 0)
+				return;
+
 			Vfs::Vfs_handle *file;
 			if (root_dir.open(name.string(),
 					          Vfs::Directory_service::OPEN_MODE_RDONLY,

repos/ports/src/noux/syscall.cc

@@ -266,69 +266,38 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 			break;
 
 		case SYSCALL_EXECVE:
-			{
+
+			typedef Child_env<sizeof(_sysio.execve_in.args)> Execve_child_env;
+
+			try {
+
+				Execve_child_env child_env(_sysio.execve_in.filename,
+					                       _sysio.execve_in.args,
+					                       _sysio.execve_in.env,
+					                       _root_dir, _vfs_io_waiter_registry,
+					                       _env.ram(), _env.rm(), _heap);
+
+				_parent_execve.execve_child(*this,
+					                        child_env.binary_name(),
+					                        child_env.args(),
+					                        child_env.env());
+
 				/*
-				 * We have to check the dataspace twice because the binary
+				 * 'return' instead of 'break' to skip possible signal delivery,
-				 * could be a script that uses an interpreter which maybe
+				 * which might cause the old child process to exit itself
-				 * does not exist.
 				 */
-				Genode::Reconstructible<Vfs_dataspace> binary_ds {
+				return true;
-					_root_dir, _vfs_io_waiter_registry,
-					_sysio.execve_in.filename, _env.ram(), _env.rm(), _heap
-				};
-
-				if (!binary_ds->ds.valid()) {
-					_sysio.error.execve = Sysio::EXECVE_NONEXISTENT;
-					break;
-				}
-
-				Child_env<sizeof(_sysio.execve_in.args)>
-					child_env(_env.rm(),
-					          _sysio.execve_in.filename, binary_ds->ds,
-					          _sysio.execve_in.args, _sysio.execve_in.env);
-
-				binary_ds.construct(_root_dir, _vfs_io_waiter_registry,
-				                    child_env.binary_name(), _env.ram(),
-				                    _env.rm(), _heap);
-
-				if (!binary_ds->ds.valid()) {
-					_sysio.error.execve = Sysio::EXECVE_NONEXISTENT;
-					break;
-				}
-
-				{
-					Attached_dataspace attached_binary_ds(_env.rm(), binary_ds->ds);
-					char const *binary_addr = attached_binary_ds.local_addr<char const>();
-					if ((binary_addr[0] != 0x7f) ||
-					    (binary_addr[1] != 'E') ||
-					    (binary_addr[2] != 'L') ||
-					    (binary_addr[3] != 'F')) {
-						_sysio.error.execve = Sysio::EXECVE_NOEXEC;
-						break;
-					}
-				}
-
-				binary_ds.destruct();
-
-				try {
-					_parent_execve.execve_child(*this,
-					                            child_env.binary_name(),
-					                            child_env.args(),
-					                            child_env.env());
-
-					/*
-					 * 'return' instead of 'break' to skip possible signal delivery,
-					 * which might cause the old child process to exit itself
-					 */
-					return true;
-				}
-				catch (Child::Binary_does_not_exist) {
-					_sysio.error.execve = Sysio::EXECVE_NONEXISTENT; }
-				catch (Child::Insufficient_memory) {
-					_sysio.error.execve = Sysio::EXECVE_NOMEM; }
-
-				break;
 			}
+			catch (Execve_child_env::Binary_does_not_exist) {
+				_sysio.error.execve = Sysio::EXECVE_ERR_NO_ENTRY; }
+			catch (Execve_child_env::Binary_is_not_accessible) {
+				_sysio.error.execve = Sysio::EXECVE_ERR_ACCESS; }
+			catch (Execve_child_env::Binary_is_not_executable) {
+				_sysio.error.execve = Sysio::EXECVE_ERR_NO_EXEC; }
+			catch (Child::Insufficient_memory) {
+				_sysio.error.execve = Sysio::EXECVE_ERR_NO_MEMORY; }
+
+			break;
 
 		case SYSCALL_SELECT:
 			{