mini-(Docker)-shell

Question

I was given an assignment to write a mini-shell:

To write your own shell, you will need to start with a C program that will prompt the user for input and accept in a number of arguments from a command line. The commands entered will be accepted into your shell and then processed to understand if it is a built in command or something that needs to be executed via fork/exec (NOTE: No use of system function). Your shell should emulate the standard shells in how it deals with background commands ( & ). The interaction with your shell should be just like the standard shells. What I mean by this is to have a good usage statement returned if the arguments passed to the shell are not correct, and when there is an error you should send back a useful error, and not exit the shell, just continue.

Namespaces allow for virtualization and sharing of spaces between parent and child processes. This is a part of the Linux operating system since 2008 that allows for the creation of different models to create containers for software applications. The most popular version of Linux namespaces is Docker. Your task for this lab is to create the option inside your shell through "built in" commands to move your shell into a container. The options for different containers can be added together in a clone or clone2 call. Here are some of the options:

• CLONE_NEWIPC - New namespace for IPC
• CLONE_NEWUTS - Setup new hostname and domain
• CLONE_NEWUSER - User and group changes
• CLONE_NEWNET - New network namespace
• CLONE_NEWNS - New mount namespace

When using a clone function, you will have the ability to run another function. To test your clone call, you will need to be able to demonstrate the change, and the best way to do that is to spawn another shell to "look around" at what changed. The best way to do this is to spawn another shell using the system function call.

Here is my code implementing the above. Basic testing has been performed and it seems to work as intended:

#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/wait.h>
#define _GNU_SOURCE
#include <linux/sched.h>
#ifndef streq
#define streq(x, y) (strcmp((x), (y)) == 0)
#endif
#define MAX_COMMAND_LENGTH 100
#define MAX_PARAMS 10
typedef struct input
{
 char** params;
 int numParams;
 bool inBackground;
} Command;
// split input into array of parameters
Command* parseCmd(char* input)
{
 // handle bad input
 if (!input || strlen(input) == 0)
 return NULL;
 // allocate space on heap for command struct, and parameter list
 Command *cmd = calloc(1, sizeof(Command));
 cmd->params = calloc(MAX_PARAMS + 1, sizeof(char*));
 // setup strtok_r
 char *next = NULL;
 char *part = strtok_r(input, " ", &next);
 // do-while so we can process first `part`, then rest of `input`
 do
 {
 // check for background, if not found default setting is false
 if (streq(part, "&"))
 {
 cmd->inBackground = true;
 // don't store in parameter list for commands
 continue;
 }
 cmd->params[cmd->numParams] = part;
 ++cmd->numParams;
 }
 while ((part = strtok_r(NULL, " ", &next)));
 return cmd;
}
void freeCmd(Command* cmd)
{
 free(cmd->params);
 cmd->params = NULL;
 free(cmd);
 cmd = NULL;
}
int childFunct(void)
{
 system("sh");
 return 0;
}
int executeClone(Command *cmd)
{
 const int STACK_SIZE = 1024;
 unsigned long flags = 0;
 if (cmd->numParams <= 1)
 {
 fprintf(stderr, "clone command requires arguments\n");
 return 0; // non-fatal error
 }
 if (streq(cmd->params[1], "ipc"))
 flags |= CLONE_NEWIPC;
 if (streq(cmd->params[1], "uts"))
 flags |= CLONE_NEWUTS;
 if (streq(cmd->params[1], "user"))
 flags |= CLONE_NEWUSER;
 if (streq(cmd->params[1], "net"))
 flags |= CLONE_NEWNET;
 if (streq(cmd->params[1], "ns"))
 flags |= CLONE_NEWNS;
 if (!flags)
 {
 fprintf(stderr, "clone parameter not recognized\n");
 return 0; // non-fatal error
 }
 char *stack = malloc(STACK_SIZE);
 pid_t pid = clone(childFunct, stack + STACK_SIZE, flags, NULL);
 // error
 if (pid == -1)
 {
 char* error = strerror(errno);
 fprintf(stderr, "clone: %s\n", error);
 }
 return 0;
}
int executeCmd(Command *cmd)
{
 pid_t pid = fork();
 // error
 if (pid == -1)
 {
 char* error = strerror(errno);
 fprintf(stderr, "fork: %s\n", error);
 return 1;
 }
 // child process
 else if (pid == 0)
 {
 // will daemonize child if background
 // if (cmd->inBackground)
 // setpgid(pid, 0);
 // execute command
 execvp(cmd->params[0], cmd->params);
 // error occurred
 char* error = strerror(errno);
 fprintf(stderr, "ERROR: %s: %s\n", cmd->params[0], error);
 return 2;
 }
 // parent process
 else
 {
 // wait for child process to finish
 if (!cmd->inBackground)
 {
 int childStatus = 0;
 waitpid(pid, &childStatus, 0);
 }
 }
 return 0;
}
int main(void)
{
 char input[MAX_COMMAND_LENGTH + 1] = "";
 Command *cmd = NULL;
 while(1)
 {
 // print command prompt
 fputs("> ", stdout);
 // fgets returns NULL on Ctrl+D, so exit the loop on NULL
 if (!fgets(input, sizeof(input), stdin))
 break;
 // remove trailing newline character, if any
 if (input[strlen(input) - 1] == '\n')
 input[strlen(input) - 1] = '0円';
 // split cmd into array of parameters
 cmd = parseCmd(input);
 // handle bad command, continue running
 if (!cmd)
 continue;
 // exit?
 if (streq(cmd->params[0], "exit") || streq(cmd->params[0], "quit"))
 break;
 // test for built-in command
 if (streq(cmd->params[0], "clone"))
 {
 if (executeClone(cmd))
 break;
 }
 else // executable
 {
 if (executeCmd(cmd))
 break;
 }
 freeCmd(cmd);
 }
}

Any suggestions for improvement?

Answer 1

1. Avoid a hacker exploit. The null character is not special when fgets() reads it. So if the first character read is the rare null character, then input[strlen(input) - 1] is UB. strcspn() is a nice alternative.

   if (!fgets(input, sizeof(input), stdin)) break;
   //if (input[strlen(input) - 1] == '\n')
   // input[strlen(input) - 1] = '0円';
   input[strcspn(input, "\n")] = '0円';
   

2. Memory leak in main(). Better to freeCmd(cmd) after the various breaks.

    freeCmd(cmd);
    }
    // add 
    freeCmd(cmd);
   }
   

3. Unclear why code is using long for flags. linux.die.net implies the type should be int. Check your prototype for clone() - also its return type. Is it int or pid_t Ref?

4. Although code nicely aligns, I'd expect an if() ... else if() ...else ... structure rather than if() ... if() ...

   if (streq(cmd->params[1], "ipc"))
    flags |= CLONE_NEWIPC;
   else /* add */ if (streq(cmd->params[1], "uts"))
    flags |= CLONE_NEWUTS;
   else /* add */ if (streq(cmd->params[1], "user"))
    flags |= CLONE_NEWUSER;
   else {
    fprintf(stderr, "clone parameter not recognized\n");
    return 0; // non-fatal error
   }
   

5. Why try to synchronize the type? Instead reference the type. Pedantic, avoid UB of signed overflow, use unsigned math with 1u.

   // Is this the right type? 
   // cmd->params = calloc(MAX_PARAMS + 1, sizeof(char*));
   // Right size, regardless of type of *cmd->params
   cmd->params = calloc(MAX_PARAMS + 1u, sizeof *(cmd->params));
   

• syb0rg: Aren't parameters not "stackable" when you change to the if-else construct?
• chux: @syb0rg Comment is unclear for me -please rephrase - will check later.
• syb0rg: My bad, I misunderstood a part of your review.

Answer 2

• Background task handling

  You must wait for the child process regardless of how it is started, in the background or not. As written, a background command ends up in the zombie state. To let the parent shell to continue, set up a signal handler for SIGCHLD, and wait there.

  I know the program statement doesn't require it, but it always nice to let user query a return status of last command.

• command("sh")

  It is unclear from the program statement, which shell should be cloned. In any case, it is highly recommended to supply a full path to system.

• freeCmd

  cmd->params = NULL and cmd = NULL are meaningless: cmd = NULL is invisible to caller, and caller shall not touch params of a freed cmd anyway.

• parseCmd

  The code recognizes & anywhere within a command line as a background indicator. It doesn't feel right for many reasons: compatibility with existing shells; & could be valid character in a filename; etc. I recommend to test for a last argument being & after parsing is done, and adjust the command appropriately.

• Built-in commands

  Is clone the only built-in? If so, your shell is at least unable to cd.

• Misc

  • You may want to fflush(stdout) after printing the prompt.

  • Test what malloc returns.

  • It is unclear whether the command line may have quotes and escapes. They are obviously not addressed.

Edit: handling background termination

In a nutshell it seems is fairly simple: define a function

void handle_child_exit(int signo)
{
 int status;
 wait(&status);
 signal(SIGCHLD, handle_child_exit); // Not always required, but helpful
}

and in install it in main:

 signal(SIGCHLD, handle_child_exit);

The reality is a bit hairier, and I can only recommend reading man signal and man sigaction.

• syb0rg: Could you give an example of how you would set up a signal handler for SIGCHLD?
• vnp: @syb0rg See edit.
• syb0rg: Why not just install signal(SIGCHLD, SIG_IGN) in main and leave what I have? Is there an advantage to this method?
• vnp: @syb0rg As written, only the foreground command is waited for. On the other hand, you cannot wait on a background command synchronously - it will defeat the purpose of backgrounding - so it is better to do it in a signal handler.
• chux: Disagree with "cmd->params = NULL and cmd = NULL are meaningless: cmd = NULL is invisible to caller, and caller shall not touch params of a freed cmd anyway." Setting pointers and data that are going to be part of free'd memory, in my experience, more readily exposes mis-use of free'd memory more rapidly than not. A net debugging benefit vs potential incurred problems. cmd = NULL; after free(cmd); is also defensive coding should subsequent code inadvertently use cmd as again, more likely to catch coding problems. Yet agree that it adds no correct code benefit.