[PATCH 1/2] Close file descriptors when running ntpdate (#1083978)

Vratislav Podzimek vpodzime at redhat.com
Thu Dec 4 14:37:14 UTC 2014


On Thu, 2014-12-04 at 09:29 -0500, Colin Walters wrote:
> On Thu, Dec 4, 2014, at 09:23 AM, Vratislav Podzimek wrote:
> 
> > +    # using iutil.execWithRedirect here would mean that if this function was run
> > +    # multiple times from multiple threads in the same time, the execution would
> > +    # be serialized because of the program_log lock
> > +    proc = subprocess.Popen(["ntpdate", "-q", server], close_fds=True)
> 
> - Why not subprocess.call()?  (You should be able to pass close_fds=True to it)
I doesn't seem to have that parameter.

> 
> - Is it intentional that you're dropping the equivalent of stdout=open('/dev/null', 'w'), stderr=subprocess.STDOUT ?
Ooops, good catch, thanks. Fixing locally.

> 
> - Also worth mentioning this fixes a bug where if the provided server name had shell metacharacters it would fail by executing code instead of returning a clean error
In general yes, but we check server name to be a valid (and even
resolvable) server name before passing it to this function.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list