Skip to content

Commit f1e87f6

Browse files
chejianjlijinxia
authored andcommitted
dm: vrtc: use signalfd to poll signal from timer
In the current implementation sigev_notify is configured as SIGEV_THREAD. When timer expires an async thread is created and the registered timer callback is called in the context of this thread. vrtc_update_timer will access the global data vrtc. There is a race condition that vrtc is freed when deinit and then a timer expires. In this case vrtc_update_timer will access a freed buffer which causes problem such as heap corruption and segment fault. In this patch signal model is used when timer is created. The signal is masked and a signalfd is used to poll on it with mevent. This avoids the race condition. Signed-off-by: Jian Jun Chen <jian.jun.chen@intel.com> Acked-by: Yin Fengwei <fengwei.yin@intel.com> Tracked-On: #1185
1 parent bcaede0 commit f1e87f6

File tree

1 file changed

+107
-20
lines changed
  • devicemodel/hw/platform

1 file changed

+107
-20
lines changed

devicemodel/hw/platform/rtc.c

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,17 @@
3333
#include <stdlib.h>
3434
#include <signal.h>
3535
#include <time.h>
36+
#include <fcntl.h>
37+
#include <unistd.h>
38+
#include <errno.h>
39+
#include <sys/syscall.h>
40+
#include <sys/signalfd.h>
3641

3742
#include "vmmapi.h"
3843
#include "inout.h"
3944
#include "mc146818rtc.h"
4045
#include "rtc.h"
46+
#include "mevent.h"
4147

4248
/* #define DEBUG_RTC */
4349
#ifdef DEBUG_RTC
@@ -70,6 +76,8 @@ struct rtcdev {
7076
struct vrtc {
7177
struct vmctx *vm;
7278
pthread_mutex_t mtx;
79+
int signal_fd;
80+
struct mevent *mevp;
7381
timer_t periodic_timer_id; /*periodic timer id*/
7482
timer_t update_timer_id; /*update timer id*/
7583
u_int addr; /* RTC register to read or write */
@@ -115,6 +123,9 @@ struct clktime {
115123
*/
116124
#define VRTC_BROKEN_TIME ((time_t)-1)
117125

126+
/* signo of timers created by vrtc */
127+
#define VRTC_TIMER_SIGNO SIGALRM
128+
118129
#define RTC_IRQ 8
119130
#define RTCSB_BIN 0x04
120131
#define RTCSB_ALL_INTRS (RTCSB_UINTR | RTCSB_AINTR | RTCSB_PINTR)
@@ -564,30 +575,29 @@ rtc_to_secs(struct vrtc *vrtc)
564575
return VRTC_BROKEN_TIME;
565576
}
566577

567-
static timer_t
568-
vrtc_create_timer(struct vrtc *vrtc, time_t sec, time_t nsec, void (*cb)())
578+
static void
579+
vrtc_create_timer(timer_t *timer_id, time_t sec, time_t nsec)
569580
{
570-
timer_t timerid;
571581
struct sigevent sigevt;
572582
struct itimerspec ts;
573583

574584
memset(&sigevt, 0, sizeof(struct sigevent));
575585
memset(&ts, 0, sizeof(struct itimerspec));
576586

577-
sigevt.sigev_value.sival_ptr = vrtc;
578-
sigevt.sigev_notify = SIGEV_THREAD;
579-
sigevt.sigev_notify_function = cb;
587+
/* there is no glibc wrapper for gettid */
588+
sigevt._sigev_un._tid = syscall(SYS_gettid);
589+
sigevt.sigev_value.sival_ptr = timer_id;
590+
sigevt.sigev_notify = SIGEV_THREAD_ID | SIGEV_SIGNAL;
591+
sigevt.sigev_signo = VRTC_TIMER_SIGNO;
580592

581-
assert(timer_create(CLOCK_REALTIME, &sigevt, &timerid) == 0);
593+
assert(timer_create(CLOCK_REALTIME, &sigevt, timer_id) == 0);
582594
/*setting the interval time*/
583595
ts.it_interval.tv_sec = sec;
584596
ts.it_interval.tv_nsec = nsec;
585597
/*set the delay time it will be started when timer_setting*/
586598
ts.it_value.tv_sec = sec;
587599
ts.it_value.tv_nsec = nsec;
588-
assert(timer_settime(timerid, 0, &ts, NULL) == 0);
589-
590-
return timerid;
600+
assert(timer_settime(*timer_id, 0, &ts, NULL) == 0);
591601
}
592602

593603
static void
@@ -811,8 +821,7 @@ vrtc_set_reg_b(struct vrtc *vrtc, uint8_t newval)
811821
}
812822

813823
/*create the new periodic timer*/
814-
vrtc->periodic_timer_id =
815-
vrtc_create_timer(vrtc, 0, newfreq, vrtc_periodic_timer);
824+
vrtc_create_timer(&vrtc->periodic_timer_id, 0, newfreq);
816825
assert(vrtc->periodic_timer_id > 0);
817826

818827
} else {
@@ -873,8 +882,7 @@ vrtc_set_reg_a(struct vrtc *vrtc, uint8_t newval)
873882
}
874883

875884
/*create the new periodic timer*/
876-
vrtc->periodic_timer_id =
877-
vrtc_create_timer(vrtc, 0, newfreq, vrtc_periodic_timer);
885+
vrtc_create_timer(&vrtc->periodic_timer_id, 0, newfreq);
878886
assert(vrtc->periodic_timer_id > 0);
879887
} else {
880888
/*Nothing to do*/
@@ -1100,12 +1108,49 @@ vrtc_enable_localtime(int l_time)
11001108
local_time = l_time;
11011109
}
11021110

1111+
static void
1112+
vrtc_handle_timer(int fd __attribute__((unused)),
1113+
enum ev_type t __attribute__((unused)),
1114+
void *arg)
1115+
{
1116+
struct vrtc *vrtc = arg;
1117+
struct signalfd_siginfo info;
1118+
timer_t *timer_id;
1119+
int len;
1120+
1121+
while (1) {
1122+
len = read(vrtc->signal_fd, &info, sizeof(info));
1123+
if (len == -1 && errno == EAGAIN)
1124+
break;
1125+
if (len == -1) {
1126+
RTC_DEBUG("signal_fd read failed: error = %d\n",
1127+
errno);
1128+
break;
1129+
}
1130+
if (len != sizeof(info)) {
1131+
RTC_DEBUG("signal_fd read len error: len = %d\n",
1132+
len);
1133+
break;
1134+
}
1135+
1136+
timer_id = (timer_t *)info.ssi_ptr;
1137+
if (*timer_id == vrtc->update_timer_id)
1138+
vrtc_update_timer(vrtc);
1139+
else if (*timer_id == vrtc->periodic_timer_id)
1140+
vrtc_periodic_timer(vrtc);
1141+
else
1142+
RTC_DEBUG("unsupported timer_id 0xlx\n", *timer_id);
1143+
}
1144+
}
1145+
11031146
int
11041147
vrtc_init(struct vmctx *ctx)
11051148
{
11061149
struct vrtc *vrtc;
11071150
struct rtcdev *rtc;
11081151
time_t curtime;
1152+
sigset_t mask;
1153+
int flags;
11091154
struct inout_port rtc_addr, rtc_data;
11101155

11111156
vrtc = calloc(1, sizeof(struct vrtc));
@@ -1115,11 +1160,6 @@ vrtc_init(struct vmctx *ctx)
11151160

11161161
pthread_mutex_init(&vrtc->mtx, NULL);
11171162

1118-
/*create update interrupt timer(1s)*/
1119-
vrtc->update_timer_id =
1120-
vrtc_create_timer(vrtc, 1, 0, vrtc_update_timer);
1121-
assert(vrtc->update_timer_id > 0);
1122-
11231163
memset(&rtc_addr, 0, sizeof(struct inout_port));
11241164
memset(&rtc_data, 0, sizeof(struct inout_port));
11251165
/*register io port handler for rtc addr*/
@@ -1162,6 +1202,31 @@ vrtc_init(struct vmctx *ctx)
11621202
secs_to_rtc(curtime, vrtc, 0);
11631203
pthread_mutex_unlock(&vrtc->mtx);
11641204

1205+
/*block VRTC_TIMER_SIGNO so it can be polled by signalfd*/
1206+
sigemptyset(&mask);
1207+
sigaddset(&mask, VRTC_TIMER_SIGNO);
1208+
pthread_sigmask(SIG_BLOCK, &mask, NULL);
1209+
1210+
/*create signalfd*/
1211+
vrtc->signal_fd = signalfd(-1, &mask, 0);
1212+
assert(vrtc->signal_fd > 0);
1213+
1214+
/*set close-on-exec*/
1215+
flags = fcntl(vrtc->signal_fd, F_GETFD);
1216+
fcntl(vrtc->signal_fd, F_SETFD, flags | FD_CLOEXEC);
1217+
1218+
/*set non-block*/
1219+
flags = fcntl(vrtc->signal_fd, F_GETFL);
1220+
fcntl(vrtc->signal_fd, F_SETFL, flags | O_NONBLOCK);
1221+
1222+
vrtc->mevp = mevent_add(vrtc->signal_fd, EVF_READ,
1223+
vrtc_handle_timer, vrtc);
1224+
assert(vrtc->mevp != NULL);
1225+
1226+
/*create update interrupt timer(1s)*/
1227+
vrtc_create_timer(&vrtc->update_timer_id, 1, 0);
1228+
assert(vrtc->update_timer_id > 0);
1229+
11651230
return 0;
11661231
}
11671232

@@ -1170,6 +1235,29 @@ vrtc_deinit(struct vmctx *ctx)
11701235
{
11711236
struct vrtc *vrtc = ctx->vrtc;
11721237
struct inout_port iop;
1238+
sig_t prev_sig_handler;
1239+
1240+
/*delete timer*/
1241+
if (vrtc->update_timer_id > 0) {
1242+
vrtc_delete_timer(vrtc->update_timer_id);
1243+
vrtc->update_timer_id = (timer_t)-1;
1244+
}
1245+
1246+
if (vrtc->periodic_timer_id > 0) {
1247+
vrtc_delete_timer(vrtc->periodic_timer_id);
1248+
vrtc->periodic_timer_id = (timer_t)-1;
1249+
}
1250+
1251+
if (vrtc->mevp)
1252+
mevent_delete(vrtc->mevp);
1253+
1254+
if (vrtc->signal_fd > 0)
1255+
close(vrtc->signal_fd);
1256+
1257+
/*clear pending signals*/
1258+
prev_sig_handler = signal(VRTC_TIMER_SIGNO, SIG_IGN);
1259+
if (prev_sig_handler != SIG_ERR)
1260+
signal(VRTC_TIMER_SIGNO, prev_sig_handler);
11731261

11741262
memset(&iop, 0, sizeof(struct inout_port));
11751263
iop.name = "rtc";
@@ -1183,7 +1271,6 @@ vrtc_deinit(struct vmctx *ctx)
11831271
iop.size = 1;
11841272
unregister_inout(&iop);
11851273

1186-
vrtc_delete_timer(vrtc->update_timer_id);
11871274
free(vrtc);
11881275
ctx->vrtc = NULL;
11891276
}

0 commit comments

Comments
 (0)