From 8aef80f978a400123f5391fe53691f99778d6d24 Mon Sep 17 00:00:00 2001
From: Jianfeng Jiang <jiangjianfeng.jjf@antgroup.com>
Date: Wed, 16 Aug 2023 17:39:27 +0800
Subject: [PATCH] Return correct error type if executing a file that is not
 executable

---
 services/libs/jinux-std/src/fs/utils/inode.rs |  4 +++
 .../src/process/program_loader/mod.rs         | 19 +++++++++-
 services/libs/jinux-std/src/syscall/execve.rs | 35 +++++++------------
 3 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/services/libs/jinux-std/src/fs/utils/inode.rs b/services/libs/jinux-std/src/fs/utils/inode.rs
index 1044fa0e9..310a58314 100644
--- a/services/libs/jinux-std/src/fs/utils/inode.rs
+++ b/services/libs/jinux-std/src/fs/utils/inode.rs
@@ -45,6 +45,10 @@ impl InodeType {
     pub fn is_reguler_file(&self) -> bool {
         *self == InodeType::File
     }
+
+    pub fn is_directory(&self) -> bool {
+        *self == InodeType::Dir
+    }
 }
 
 impl From<DeviceType> for InodeType {
diff --git a/services/libs/jinux-std/src/process/program_loader/mod.rs b/services/libs/jinux-std/src/process/program_loader/mod.rs
index ec5796b19..a3c5ee11b 100644
--- a/services/libs/jinux-std/src/process/program_loader/mod.rs
+++ b/services/libs/jinux-std/src/process/program_loader/mod.rs
@@ -35,7 +35,7 @@ pub fn load_program_to_root_vmar(
     };
     if let Some(mut new_argv) = parse_shebang_line(&*file_header)? {
         if recursion_limit == 0 {
-            return_errno_with_message!(Errno::EINVAL, "the recursieve limit is reached");
+            return_errno_with_message!(Errno::ELOOP, "the recursieve limit is reached");
         }
         new_argv.extend_from_slice(&argv);
         let interpreter = {
@@ -43,6 +43,7 @@ pub fn load_program_to_root_vmar(
             let fs_path = FsPath::new(AT_FDCWD, &filename)?;
             fs_resolver.lookup(&fs_path)?
         };
+        check_executable_file(&interpreter)?;
         return load_program_to_root_vmar(
             root_vmar,
             interpreter,
@@ -56,3 +57,19 @@ pub fn load_program_to_root_vmar(
         load_elf_to_root_vmar(root_vmar, &*file_header, elf_file, fs_resolver, argv, envp)?;
     Ok((abs_path, elf_load_info))
 }
+
+pub fn check_executable_file(dentry: &Arc<Dentry>) -> Result<()> {
+    if dentry.inode_type().is_directory() {
+        return_errno_with_message!(Errno::EISDIR, "the file is a directory");
+    }
+
+    if !dentry.inode_type().is_reguler_file() {
+        return_errno_with_message!(Errno::EACCES, "the dentry is not a regular file");
+    }
+
+    if !dentry.inode_mode().is_executable() {
+        return_errno_with_message!(Errno::EACCES, "the dentry is not executable");
+    }
+
+    Ok(())
+}
diff --git a/services/libs/jinux-std/src/syscall/execve.rs b/services/libs/jinux-std/src/syscall/execve.rs
index 7f0b1ec7c..a89bfcb9f 100644
--- a/services/libs/jinux-std/src/syscall/execve.rs
+++ b/services/libs/jinux-std/src/syscall/execve.rs
@@ -8,7 +8,7 @@ use crate::log_syscall_entry;
 use crate::prelude::*;
 use crate::process::posix_thread::name::ThreadName;
 use crate::process::posix_thread::posix_thread_ext::PosixThreadExt;
-use crate::process::program_loader::load_program_to_root_vmar;
+use crate::process::program_loader::{check_executable_file, load_program_to_root_vmar};
 use crate::syscall::{SYS_EXECVE, SYS_EXECVEAT};
 use crate::util::{read_cstring_from_user, read_val_from_user};
 
@@ -19,13 +19,11 @@ pub fn sys_execve(
     context: &mut UserContext,
 ) -> Result<SyscallReturn> {
     log_syscall_entry!(SYS_EXECVE);
-    let executable_path = read_filename(filename_ptr)?;
     let elf_file = {
-        let current = current!();
-        let fs_resolver = current.fs().read();
-        let fs_path = FsPath::new(AT_FDCWD, &executable_path)?;
-        fs_resolver.lookup(&fs_path)?
+        let executable_path = read_filename(filename_ptr)?;
+        lookup_executable_file(AT_FDCWD, executable_path, OpenFlags::empty())?
     };
+
     do_execve(elf_file, argv_ptr_ptr, envp_ptr_ptr, context)?;
     Ok(SyscallReturn::NoReturn)
 }
@@ -39,10 +37,13 @@ pub fn sys_execveat(
     context: &mut UserContext,
 ) -> Result<SyscallReturn> {
     log_syscall_entry!(SYS_EXECVEAT);
-    let flags = OpenFlags::from_bits_truncate(flags);
-    let filename = read_filename(filename_ptr)?;
-    let elf_file = lookup_executable_file(dfd, filename, flags)?;
-    check_file_type_and_permission(&elf_file)?;
+
+    let elf_file = {
+        let flags = OpenFlags::from_bits_truncate(flags);
+        let filename = read_filename(filename_ptr)?;
+        lookup_executable_file(dfd, filename, flags)?
+    };
+
     do_execve(elf_file, argv_ptr_ptr, envp_ptr_ptr, context)?;
     Ok(SyscallReturn::NoReturn)
 }
@@ -68,22 +69,10 @@ fn lookup_executable_file(
             fs_resolver.lookup(&fs_path)
         }
     }?;
-    check_file_type_and_permission(&dentry)?;
+    check_executable_file(&dentry)?;
     Ok(dentry)
 }
 
-fn check_file_type_and_permission(dentry: &Arc<Dentry>) -> Result<()> {
-    if !dentry.inode_type().is_reguler_file() {
-        return_errno_with_message!(Errno::EACCES, "the dentry is not a regular file");
-    }
-
-    if !dentry.inode_mode().is_executable() {
-        return_errno_with_message!(Errno::EACCES, "the dentry is not executable");
-    }
-
-    Ok(())
-}
-
 fn do_execve(
     elf_file: Arc<Dentry>,
     argv_ptr_ptr: Vaddr,