[PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices

Julio Faracco posted 2 patches 5 years, 11 months ago
[PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Julio Faracco 5 years, 11 months ago
This commit share host Real Time Clock device (rtc) into LXC containers
to support hardware clock. This should be available setting up a `rtc`
timer under clock section. Since this option is not emulated, it should
be available only for `localtime` clock. This option should be readonly
due to security reasons.

Before:
    root# hwclock --verbose
    hwclock from util-linux 2.32.1
    System Time: 1581877557.598365
    Trying to open: /dev/rtc0
    Trying to open: /dev/rtc
    Trying to open: /dev/misc/rtc
    No usable clock interface found.
    hwclock: Cannot access the Hardware Clock via any known method.

Now:
    root# hwclock
    2020-02-16 18:23:55.374134+00:00
    root# hwclock -w
    hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
    Permission denied

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 docs/formatdomain.html.in |  2 +-
 src/lxc/lxc_cgroup.c      | 33 ++++++++++++++++++++
 src/lxc/lxc_controller.c  | 66 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 28770188dd..96d06e9c3e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2465,7 +2465,7 @@
             being modified, and can be one of
             "platform" (currently unsupported),
             "hpet" (libxl, xen, qemu), "kvmclock" (qemu),
-            "pit" (qemu), "rtc" (qemu), "tsc" (libxl, qemu -
+            "pit" (qemu), "rtc" (qemu, lxc), "tsc" (libxl, qemu -
             <span class="since">since 3.2.0</span>), "hypervclock"
             (qemu - <span class="since">since 1.2.2</span>) or
             "armvtimer" (qemu - <span class="since">since 6.1.0</span>).
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 952d021418..065996b3a4 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -333,6 +333,39 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
                              VIR_CGROUP_DEVICE_RWM) < 0)
         return -1;
 
+    VIR_DEBUG("Allowing timers char devices");
+
+    /* Sync'ed with Host clock */
+    for (i = 0; i < def->clock.ntimers; i++) {
+        virDomainTimerDefPtr timer = def->clock.timers[i];
+
+        /* Check if "present" is set to "no" otherwise enable it. */
+        if (!timer->present)
+            continue;
+
+        switch ((virDomainTimerNameType)timer->name) {
+        case VIR_DOMAIN_TIMER_NAME_PLATFORM:
+        case VIR_DOMAIN_TIMER_NAME_TSC:
+        case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
+        case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
+        case VIR_DOMAIN_TIMER_NAME_PIT:
+        case VIR_DOMAIN_TIMER_NAME_HPET:
+        case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
+        case VIR_DOMAIN_TIMER_NAME_LAST:
+            break;
+        case VIR_DOMAIN_TIMER_NAME_RTC:
+            if (virFileExists("/dev/rtc")) {
+                if (virCgroupAllowDevicePath(cgroup, "/dev/rtc",
+                                             VIR_CGROUP_DEVICE_READ,
+                                             false) < 0)
+                    return -1;
+            } else {
+                VIR_DEBUG("Ignoring non-existent device /dev/rtc");
+            }
+            break;
+        }
+    }
+
     VIR_DEBUG("Device whitelist complete");
 
     return 0;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index fa7b9dd0e7..d97c4ad1b5 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1530,6 +1530,69 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl)
 }
 
 
+static int
+virLXCControllerSetupTimers(virLXCControllerPtr ctrl)
+{
+    g_autofree char *path = NULL;
+    size_t i;
+    struct stat sb;
+    virDomainDefPtr def = ctrl->def;
+    const char *rtc_dev = "/dev/rtc";
+
+    /* Not sync'ed with Host clock */
+    if (def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME)
+        return 0;
+
+    for (i = 0; i < def->clock.ntimers; i++) {
+        dev_t dev;
+        virDomainTimerDefPtr timer = def->clock.timers[i];
+
+        /* Check if "present" is set to "no" otherwise enable it. */
+        if (!timer->present)
+            continue;
+
+        switch ((virDomainTimerNameType)timer->name) {
+        case VIR_DOMAIN_TIMER_NAME_PLATFORM:
+        case VIR_DOMAIN_TIMER_NAME_TSC:
+        case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
+        case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
+        case VIR_DOMAIN_TIMER_NAME_PIT:
+        case VIR_DOMAIN_TIMER_NAME_HPET:
+        case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
+        case VIR_DOMAIN_TIMER_NAME_LAST:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("unsupported timer type (name) '%s'"),
+                           virDomainTimerNameTypeToString(timer->name));
+            return -1;
+        case VIR_DOMAIN_TIMER_NAME_RTC:
+            if (stat(rtc_dev, &sb) < 0) {
+                virReportSystemError(errno, _("Unable to access %s"),
+                                     rtc_dev);
+                return -1;
+            }
+
+            path = g_strdup_printf("/%s/%s.dev/%s", LXC_STATE_DIR,
+                                   ctrl->def->name, "/rtc");
+
+            dev = makedev(major(sb.st_rdev), minor(sb.st_rdev));
+            if (mknod(path, S_IFCHR, dev) < 0 ||
+                chmod(path, sb.st_mode)) {
+                virReportSystemError(errno,
+                                     _("Failed to make device %s"),
+                                     path);
+                return -1;
+            }
+
+            if (lxcContainerChown(ctrl->def, path) < 0)
+                return -1;
+            break;
+        }
+    }
+
+    return 0;
+}
+
+
 static int
 virLXCControllerSetupHostdevSubsysUSB(virDomainDefPtr vmDef,
                                       virDomainHostdevDefPtr def,
@@ -2321,6 +2384,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
     if (virLXCControllerPopulateDevices(ctrl) < 0)
         goto cleanup;
 
+    if (virLXCControllerSetupTimers(ctrl) < 0)
+        goto cleanup;
+
     if (virLXCControllerSetupAllDisks(ctrl) < 0)
         goto cleanup;
 
-- 
2.24.1


Re: [PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Michal Prívozník 5 years, 10 months ago
On 2. 3. 2020 1:54, Julio Faracco wrote:
> This commit share host Real Time Clock device (rtc) into LXC containers
> to support hardware clock. This should be available setting up a `rtc`
> timer under clock section. Since this option is not emulated, it should
> be available only for `localtime` clock. This option should be readonly
> due to security reasons.
> 
> Before:
>     root# hwclock --verbose
>     hwclock from util-linux 2.32.1
>     System Time: 1581877557.598365
>     Trying to open: /dev/rtc0
>     Trying to open: /dev/rtc
>     Trying to open: /dev/misc/rtc
>     No usable clock interface found.
>     hwclock: Cannot access the Hardware Clock via any known method.
> 
> Now:
>     root# hwclock
>     2020-02-16 18:23:55.374134+00:00
>     root# hwclock -w
>     hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
>     Permission denied
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  docs/formatdomain.html.in |  2 +-
>  src/lxc/lxc_cgroup.c      | 33 ++++++++++++++++++++
>  src/lxc/lxc_controller.c  | 66 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 100 insertions(+), 1 deletion(-)

On my system, the /dev/rtc is just a symlink to /dev/rtc0. Should we
create the symlink too or is it okay to just create /dev/rtc?

Otherwise looking good.

Michal

Re: [PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Julio Faracco 5 years, 10 months ago
Hi Michal,

/dev/rtc symlink is created by udev default rules.
Maybe we can check for both: /dev/rtc and /dev/rtc0.

--
Julio Cesar Faracco

Em seg., 16 de mar. de 2020 às 15:16, Michal Prívozník
<mprivozn@redhat.com> escreveu:
>
> On 2. 3. 2020 1:54, Julio Faracco wrote:
> > This commit share host Real Time Clock device (rtc) into LXC containers
> > to support hardware clock. This should be available setting up a `rtc`
> > timer under clock section. Since this option is not emulated, it should
> > be available only for `localtime` clock. This option should be readonly
> > due to security reasons.
> >
> > Before:
> >     root# hwclock --verbose
> >     hwclock from util-linux 2.32.1
> >     System Time: 1581877557.598365
> >     Trying to open: /dev/rtc0
> >     Trying to open: /dev/rtc
> >     Trying to open: /dev/misc/rtc
> >     No usable clock interface found.
> >     hwclock: Cannot access the Hardware Clock via any known method.
> >
> > Now:
> >     root# hwclock
> >     2020-02-16 18:23:55.374134+00:00
> >     root# hwclock -w
> >     hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
> >     Permission denied
> >
> > Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> > ---
> >  docs/formatdomain.html.in |  2 +-
> >  src/lxc/lxc_cgroup.c      | 33 ++++++++++++++++++++
> >  src/lxc/lxc_controller.c  | 66 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 100 insertions(+), 1 deletion(-)
>
> On my system, the /dev/rtc is just a symlink to /dev/rtc0. Should we
> create the symlink too or is it okay to just create /dev/rtc?
>
> Otherwise looking good.
>
> Michal
>


Re: [PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Michal Prívozník 5 years, 10 months ago
On 17. 3. 2020 14:41, Julio Faracco wrote:
> Hi Michal,
> 
> /dev/rtc symlink is created by udev default rules.
> Maybe we can check for both: /dev/rtc and /dev/rtc0.

I think it's safe to rely on default udev behavior and just create rtc0.
What do you think? No need to send another version, I can do the fixes
locally before pushing.

Michal

Re: [PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Julio Faracco 5 years, 10 months ago
I'm good with that.
I verified other resources that use rtc... they are checking for rtc0.
Thanks, Michal

--
Julio Cesar Faracco

Em ter., 17 de mar. de 2020 às 12:32, Michal Prívozník
<mprivozn@redhat.com> escreveu:
>
> On 17. 3. 2020 14:41, Julio Faracco wrote:
> > Hi Michal,
> >
> > /dev/rtc symlink is created by udev default rules.
> > Maybe we can check for both: /dev/rtc and /dev/rtc0.
>
> I think it's safe to rely on default udev behavior and just create rtc0.
> What do you think? No need to send another version, I can do the fixes
> locally before pushing.
>
> Michal
>


Re: [PATCH v5 1/2] lxc: Add Real Time Clock device into allowed devices
Posted by Michal Prívozník 5 years, 10 months ago
On 17. 3. 2020 17:39, Julio Faracco wrote:
> I'm good with that.
> I verified other resources that use rtc... they are checking for rtc0.
> Thanks, Michal
> 

Cool, I've fixed patches (actually I did slightly change them to make it
easier to add new timer) and pushed.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal