Merge pull request #1270 from microsoft/bugfix-pdpix-sockopt

[libos] Disallow Setting SO_LINGER When Socket is Closing

Author: ppenna

man/demi_getsockopt.md

@@ -0,0 +1,48 @@
+# `demi_setsockopt()`
+
+## Name
+
+`demi_setsockopt` - Gets a socket option on a socket I/O queue.
+
+## Synopsis
+
+```c
+#include <demi/libos.h>
+#include <sys/socket.h> /* For SOL_SOCKET. */
+
+int demi_setsockopt(int sockqd, int level, int optname, const void *optval, socklen_t optlen);
+```
+
+## Description
+
+`demi_getsockopt()` gets the option specified by the `optname` argument, at the protocol level specified by the `level` argument, to the value pointed to by the `optval` argument for the socket I/O queue associated with queue descriptor specified by the `socketqd` argument.
+
+Currently the following values for `level` are supported:
+
+- `SOL_SOCKET` - Socket-level options.
+
+Currently the following values for `option` are supported:
+
+- `SO_LINGER` - Linger on/off and linger time in seconds, for queued, unsent data on `demi_close()`.
+
+## Return Value
+
+On success, zero is returned. On error, a positive error code is returned.
+
+## Errors
+
+On error, one of the following positive error codes is returned:
+
+- `EBADF` - The specified `sockqd` is invalid.
+- `EINVAL` - The specified `optval` is invalid.
+- `EINVAL` - The specified `optlen` is invalid.
+- `ENOPROTOOPT` - The specified `optname` is not supported.
+- `ENOTSUP` - The specified `level` is not supported.
+
+## Disclaimer
+
+Any behavior that is not documented in this manual page is unintentional and should be reported.
+
+## See Also
+
+`demi_setsockopt()`

man/demi_setsockopt.md

@@ -0,0 +1,51 @@
+# `demi_setsockopt()`
+
+## Name
+
+`demi_setsockopt` - Sets a socket option on a socket I/O queue.
+
+## Synopsis
+
+```c
+#include <demi/libos.h>
+#include <sys/socket.h> /* For SOL_SOCKET. */
+
+int demi_setsockopt(int sockqd, int level, int optname, const void *optval, socklen_t optlen);
+```
+
+## Description
+
+`demi_setsockopt()` sets the option specified by the `optname` argument, at the protocol level specified by the `level`
+argument, to the value pointed to by the `optval` argument for the socket I/O queue associated with queue descriptor
+specified by the `socketqd` argument.
+
+Currently the following values for `level` are supported:
+
+- `SOL_SOCKET` - Socket-level options.
+
+Currently the following values for `option` are supported:
+
+- `SO_LINGER` - Linger on/off and linger time in seconds, for queued, unsent data on `demi_close()`.
+
+## Return Value
+
+On success, zero is returned. On error, a positive error code is returned.
+
+## Errors
+
+On error, one of the following positive error codes is returned:
+
+- `EBADF` - The specified `sockqd` is invalid.
+- `EBUSY` - Cannot set option because socket is busy.
+- `EINVAL` - The specified `optval` is invalid.
+- `EINVAL` - The specified `optlen` is invalid.
+- `ENOPROTOOPT` - The specified `optname` is not supported.
+- `ENOTSUP` - The specified `level` is not supported.
+
+## Disclaimer
+
+Any behavior that is not documented in this manual page is unintentional and should be reported.
+
+## See Also
+
+`demi_getsockopt()`

src/rust/demikernel/bindings.rs

@@ -793,21 +793,20 @@ pub extern "C" fn demi_setsockopt(
 
     // Check inputs.
     if level != SOL_SOCKET {
-        let cause: String = format!("Only options at the socket level are supported");
-        error!("demi_setsockopt(): {}", cause);
-        return libc::EINVAL;
+        error!("demi_setsockopt(): only options in SOL_SOCKET level are supported");
+        return libc::ENOTSUP;
     }
 
     let opt: SocketOption = match optname {
         SO_LINGER => {
             // Check for invalid storage locations.
             if optval.is_null() {
-                warn!("demi_setsockopt() linger value is a null pointer");
+                error!("demi_setsockopt(): linger value is a null pointer");
                 return libc::EINVAL;
             }
 
             if optlen as usize != mem::size_of::<Linger>() {
-                warn!("demi_setsockopt() linger len is incorrect");
+                warn!("demi_setsockopt(): linger len is incorrect");
                 return libc::EINVAL;
             }
 
@@ -818,25 +817,24 @@ pub extern "C" fn demi_setsockopt(
             }
         },
         _ => {
-            let cause: String = format!("Only SO_LINGER is supported right now");
-            error!("demi_setsockopt(): {}", cause);
-            return libc::EINVAL;
+            error!("demi_setsockopt(): only SO_LINGER is supported right now");
+            return libc::ENOPROTOOPT;
         },
     };
 
     // Issue socket operation.
     let ret: Result<(), Fail> = match do_syscall(|libos| libos.set_socket_option(qd.into(), opt)) {
         Ok(result) => result,
         Err(e) => {
-            trace!("demi_getsockopt() failed: {:?}", e);
+            trace!("demi_getsockopt(): {:?}", e);
             return e.errno;
         },
     };
 
     match ret {
         Ok(_) => 0,
         Err(e) => {
-            trace!("demi_getsockopt() failed: {:?}", e);
+            trace!("demi_getsockopt(): {:?}", e);
             e.errno
         },
     }
@@ -858,36 +856,34 @@ pub extern "C" fn demi_getsockopt(
 
     // Check inputs.
     if level != SOL_SOCKET {
-        let cause: String = format!("Only options at the socket level are supported");
-        error!("demi_getsockopt(): {}", cause);
-        return libc::EINVAL;
+        error!("demi_getsockopt(): only options in SOL_SOCKET level are supported");
+        return libc::ENOTSUP;
     }
 
     let opt: SocketOption = match optname {
         SO_LINGER => SocketOption::SO_LINGER(None),
         _ => {
-            let cause: String = format!("Only SO_LINGER is supported right now");
-            error!("demi_getsockopt(): {}", cause);
-            return libc::EINVAL;
+            error!("demi_getsockopt(): only SO_LINGER is supported right now");
+            return libc::ENOPROTOOPT;
         },
     };
 
     // Check for invalid storage locations.
     if optval.is_null() {
-        warn!("demi_getsockopt() option value is a null pointer");
+        warn!("demi_getsockopt(): option value is a null pointer");
         return libc::EINVAL;
     }
 
     if optlen.is_null() {
-        warn!("demi_getsockopt() option len is a null pointer");
+        warn!("demi_getsockopt(): option len is a null pointer");
         return libc::EINVAL;
     }
 
     // Issue socket operation.
     let ret: Result<SocketOption, Fail> = match do_syscall(|libos| libos.get_socket_option(qd.into(), opt)) {
         Ok(result) => result,
         Err(e) => {
-            trace!("demi_getsockopt() failed: {:?}", e);
+            trace!("demi_getsockopt(): {:?}", e);
             return e.errno;
         },
     };
@@ -912,7 +908,7 @@ pub extern "C" fn demi_getsockopt(
                         },
                     };
 
-                    let result_length = mem::size_of::<Linger>();
+                    let result_length: usize = mem::size_of::<Linger>();
                     unsafe {
                         ptr::copy(&result as *const Linger as *const c_void, optval, result_length);
                         *optlen = result_length as Socklen;
@@ -922,7 +918,7 @@ pub extern "C" fn demi_getsockopt(
             0
         },
         Err(e) => {
-            trace!("demi_getsockopt() failed: {:?}", e);
+            trace!("demi_getsockopt(): {:?}", e);
             return e.errno;
         },
     }

src/rust/demikernel/libos/network/queue.rs

@@ -96,6 +96,18 @@ impl<T: NetworkTransport> SharedNetworkQueue<T> {
 
     /// Sets a socket option on the socket.
     pub fn set_socket_option(&mut self, option: SocketOption) -> Result<(), Fail> {
+        // Ensure that option can be set, depending on the state of the socket.
+        #[allow(unreachable_patterns)]
+        match option {
+            SocketOption::SO_LINGER(_) => {
+                if let Err(_) = self.state_machine.ensure_not_closing() {
+                    let cause: String = format!("catnnot set SO_LINGER when socket is closing");
+                    warn!("set_socket_option(): {}", cause);
+                    return Err(Fail::new(libc::EBUSY, &cause));
+                }
+            },
+            _ => (),
+        }
         self.transport.clone().set_socket_option(&mut self.socket, option)
     }
 

src/rust/runtime/network/socket/state.rs

@@ -351,7 +351,7 @@ impl SocketStateMachine {
     }
 
     /// Ensures that the target [SocketState] is not closing.
-    fn ensure_not_closing(&self) -> Result<(), Fail> {
+    pub fn ensure_not_closing(&self) -> Result<(), Fail> {
         if self.current.get() == SocketState::Closing {
             let cause: String = format!("socket is closing");
             error!("ensure_not_closing(): {}", cause);