| Message ID | 20210930153037.1194279-14-vgoyal@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtiofsd: Support notification queue and | expand |
On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() > syscall. Now these patches are making use of g_usleep(). So add > clock_nanosleep() to list of allowed syscalls. > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > --- > tools/virtiofsd/passthrough_seccomp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > index cd24b40b78..03080806c0 100644 > --- a/tools/virtiofsd/passthrough_seccomp.c > +++ b/tools/virtiofsd/passthrough_seccomp.c > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > SCMP_SYS(writev), > SCMP_SYS(umask), > SCMP_SYS(nanosleep), > + SCMP_SYS(clock_nanosleep), This patch can be dropped once sleep has been replaced by a condvar.
On Tue, Oct 05, 2021 at 01:22:58PM +0100, Stefan Hajnoczi wrote: > On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > > g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() > > syscall. Now these patches are making use of g_usleep(). So add > > clock_nanosleep() to list of allowed syscalls. > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > --- > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > index cd24b40b78..03080806c0 100644 > > --- a/tools/virtiofsd/passthrough_seccomp.c > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > > SCMP_SYS(writev), > > SCMP_SYS(umask), > > SCMP_SYS(nanosleep), > > + SCMP_SYS(clock_nanosleep), > > This patch can be dropped once sleep has been replaced by a condvar. There is another sleep in do_pool_destroy() where we are waiting for all current threads to exit. do_pool_destroy() { g_usleep(10000); } Vivek
On Tue, Oct 05, 2021 at 11:16:18AM -0400, Vivek Goyal wrote: > On Tue, Oct 05, 2021 at 01:22:58PM +0100, Stefan Hajnoczi wrote: > > On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > > > g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() > > > syscall. Now these patches are making use of g_usleep(). So add > > > clock_nanosleep() to list of allowed syscalls. > > > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > > --- > > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > > index cd24b40b78..03080806c0 100644 > > > --- a/tools/virtiofsd/passthrough_seccomp.c > > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > > > SCMP_SYS(writev), > > > SCMP_SYS(umask), > > > SCMP_SYS(nanosleep), > > > + SCMP_SYS(clock_nanosleep), > > > > This patch can be dropped once sleep has been replaced by a condvar. > > There is another sleep in do_pool_destroy() where we are waiting > for all current threads to exit. > > do_pool_destroy() { > g_usleep(10000); > } That won't be necessary if there's a way to avoid the thread pool :). See my other reply about closing the OFD instead of using signals to cancel blocking fcntl(2). Stefan
On Tue, Oct 05, 2021 at 04:50:43PM +0100, Stefan Hajnoczi wrote: > On Tue, Oct 05, 2021 at 11:16:18AM -0400, Vivek Goyal wrote: > > On Tue, Oct 05, 2021 at 01:22:58PM +0100, Stefan Hajnoczi wrote: > > > On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > > > > g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() > > > > syscall. Now these patches are making use of g_usleep(). So add > > > > clock_nanosleep() to list of allowed syscalls. > > > > > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > > > --- > > > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > > > index cd24b40b78..03080806c0 100644 > > > > --- a/tools/virtiofsd/passthrough_seccomp.c > > > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > > > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > > > > SCMP_SYS(writev), > > > > SCMP_SYS(umask), > > > > SCMP_SYS(nanosleep), > > > > + SCMP_SYS(clock_nanosleep), > > > > > > This patch can be dropped once sleep has been replaced by a condvar. > > > > There is another sleep in do_pool_destroy() where we are waiting > > for all current threads to exit. > > > > do_pool_destroy() { > > g_usleep(10000); > > } > > That won't be necessary if there's a way to avoid the thread pool :). > See my other reply about closing the OFD instead of using signals to > cancel blocking fcntl(2). Hi Stefan, I responded to that email already. man fnctl does not say anything about closing fd will unblock the waiter with -EINTR and I had a quick look at kernel code and did not find anything which suggested closing fd will unblock current waiters. So is this something you know works or you want me to try and see if it works? Thanks Vivek
On Tue, Oct 05, 2021 at 01:28:21PM -0400, Vivek Goyal wrote: > On Tue, Oct 05, 2021 at 04:50:43PM +0100, Stefan Hajnoczi wrote: > > On Tue, Oct 05, 2021 at 11:16:18AM -0400, Vivek Goyal wrote: > > > On Tue, Oct 05, 2021 at 01:22:58PM +0100, Stefan Hajnoczi wrote: > > > > On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > > > > > g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() > > > > > syscall. Now these patches are making use of g_usleep(). So add > > > > > clock_nanosleep() to list of allowed syscalls. > > > > > > > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > > > > --- > > > > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c > > > > > index cd24b40b78..03080806c0 100644 > > > > > --- a/tools/virtiofsd/passthrough_seccomp.c > > > > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > > > > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > > > > > SCMP_SYS(writev), > > > > > SCMP_SYS(umask), > > > > > SCMP_SYS(nanosleep), > > > > > + SCMP_SYS(clock_nanosleep), > > > > > > > > This patch can be dropped once sleep has been replaced by a condvar. > > > > > > There is another sleep in do_pool_destroy() where we are waiting > > > for all current threads to exit. > > > > > > do_pool_destroy() { > > > g_usleep(10000); > > > } > > > > That won't be necessary if there's a way to avoid the thread pool :). > > See my other reply about closing the OFD instead of using signals to > > cancel blocking fcntl(2). > > Hi Stefan, > > I responded to that email already. man fnctl does not say anything > about closing fd will unblock the waiter with -EINTR and I had a > quick look at kernel code and did not find anything which suggested > closing fd will unblock current waiters. > > So is this something you know works or you want me to try and see > if it works? Thanks for testing it! Stefan
diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c index cd24b40b78..03080806c0 100644 --- a/tools/virtiofsd/passthrough_seccomp.c +++ b/tools/virtiofsd/passthrough_seccomp.c @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { SCMP_SYS(writev), SCMP_SYS(umask), SCMP_SYS(nanosleep), + SCMP_SYS(clock_nanosleep), }; /* Syscalls used when --syslog is enabled */
g_usleep() calls nanosleep() and that now seems to call clock_nanosleep() syscall. Now these patches are making use of g_usleep(). So add clock_nanosleep() to list of allowed syscalls. Signed-off-by: Vivek Goyal <vgoyal@redhat.com> --- tools/virtiofsd/passthrough_seccomp.c | 1 + 1 file changed, 1 insertion(+)