OpenVZ Forum


Home » Mailing lists » Devel » vzctl: race condition at open("/sbin/init")
vzctl: race condition at open("/sbin/init") [message #47277] Wed, 25 July 2012 19:07 Go to next message
Vasily Kulikov is currently offline  Vasily Kulikov
Messages: 2
Registered: July 2012
Junior Member
From: *parallels.com
Hi,

stat()+open() is not atomic in the code below, so there is a race
condition. A container root may change /sbin/init between these calls
to e.g. FIFO and then make the vzctl's process hang up on read().

I'd add O_NOCTTY to open's flags and change stat() before open() to
fstat() just after open().


vzctl-3.3/src/lib/readelf.c:

int get_arch_from_elf(const char *file)
{
...
if (stat(file, &st)) <<<<<
return -1;
if (!S_ISREG(st.st_mode))
return -1;
fd = open(file, O_RDONLY); <<<<<
if (fd < 0)
return -1;
nbytes = read(fd, (void *) &elf_hdr, sizeof(elf_hdr));
...
}

Thanks,

--
Vasily
Re: vzctl: race condition at open(&quot;/sbin/init&quot;) [message #47917 is a reply to message #47277] Tue, 18 September 2012 15:09 Go to previous messageGo to next message
kir is currently offline  kir
Messages: 1645
Registered: August 2005
Location: Moscow, Russia
Senior Member

From: *parallels.com
On 07/25/2012 11:07 PM, Vasily Kulikov wrote:
> Hi,
>
> stat()+open() is not atomic in the code below, so there is a race
> condition. A container root may change /sbin/init between these calls
> to e.g. FIFO and then make the vzctl's process hang up on read().
>
> I'd add O_NOCTTY to open's flags and change stat() before open() to
> fstat() just after open().

Thanks a lot for reporting!

Does this patch seems sufficient to you?
http://git.openvz.org/?p=vzctl;a=commitdiff;h=7c47a7953

>
>
> vzctl-3.3/src/lib/readelf.c:
>
> int get_arch_from_elf(const char *file)
> {
> ...
> if (stat(file, &st)) <<<<<
> return -1;
> if (!S_ISREG(st.st_mode))
> return -1;
> fd = open(file, O_RDONLY); <<<<<
> if (fd < 0)
> return -1;
> nbytes = read(fd, (void *) &elf_hdr, sizeof(elf_hdr));
> ...
> }
>
> Thanks,
>


Kir Kolyshkin
http://static.openvz.org/userbars/openvz-developer.png
Re: vzctl: race condition at open(&quot;/sbin/init&quot;) [message #47924 is a reply to message #47917] Tue, 18 September 2012 20:12 Go to previous message
Vasily Kulikov is currently offline  Vasily Kulikov
Messages: 2
Registered: July 2012
Junior Member
From: *parallels.com
On Tue, Sep 18, 2012 at 19:09 +0400, Kir Kolyshkin wrote:
> On 07/25/2012 11:07 PM, Vasily Kulikov wrote:
> >Hi,
> >
> >stat()+open() is not atomic in the code below, so there is a race
> >condition. A container root may change /sbin/init between these calls
> >to e.g. FIFO and then make the vzctl's process hang up on read().
> >
> >I'd add O_NOCTTY to open's flags and change stat() before open() to
> >fstat() just after open().
>
> Thanks a lot for reporting!
>
> Does this patch seems sufficient to you?
> http://git.openvz.org/?p=vzctl;a=commitdiff;h=7c47a7953

Yes, look good.

Thanks!

--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
Previous Topic: [VZCTL PATCH] dists: add distribution config file for Alpine Linux
Next Topic: [PATCH v5 00/10] IPC: checkpoint/restore in userspace enhancements
Goto Forum:
  


Current Time: Tue Oct 16 21:34:25 GMT 2018