[jira] [Commented] (YARN-8207) Docker container launch use popen have risk of shell expansion

["Eric Yang (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-8207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16459839#comment-16459839 ] 

Eric Yang commented on YARN-8207:
---------------------------------

{quote}One problem with the foreground change being implemented here is when the docker image needs to be downloaded. The inspect command will fail until the docker container starts running, but the container will not start running until the necessary layers are downloaded. If this takes more than 10 seconds (or whatever the retries are) then it will report a failed launch yet the Docker container will eventually start when the layers finish downloading. At that point I believe the NM will have leaked a Docker container.{quote}

No leak of docker container.  Download happens before docker daemon registered creation of docker container.  If docker run is aborted prior to complete download, it does not leave partial image or container instance behind.

{quote}For the purposes of this JIRA, I think we should preserve the current behavior (i.e.: wait for the run command to return then do the inspect).{quote}

I will update the code to preserve detach and wait for this JIRA.

{quote}Cool, was this measured empirically or referenced from some article? It would be good to put a comment explaining the +2, otherwise it's likely to get "fixed" by someone coming along later and thinking it's an off-by-one error. Also I'm not sure what's meant by "recursion" here since the function is not recursive. Could you explain?{quote}

Sorry recursion is the wrong word.  The original while loop repeats for every character that doesn't fit in the buffer size for string greater than 100 characters.  By cut down the number of retries of the loop by half using twos increment of buffer size. 
 It reduces the time spent in the loop in half for strings that goes beyond 120 characters.  This is more empirically optimization, and to avoid possible copyright issue for borrowing code from GPL man page.

> Docker container launch use popen have risk of shell expansion
> --------------------------------------------------------------
>
>                 Key: YARN-8207
>                 URL: https://issues.apache.org/jira/browse/YARN-8207
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: yarn-native-services
>            Reporter: Eric Yang
>            Assignee: Eric Yang
>            Priority: Major
>         Attachments: YARN-8207.001.patch, YARN-8207.002.patch
>
>
> Container-executor code utilize a string buffer to construct docker run command, and pass the string buffer to popen for execution.  Popen spawn a shell to run the command.  Some arguments for docker run are still vulnerable to shell expansion.  The possible solution is to convert from char * buffer to string array for execv to avoid shell expansion.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org