Date: Mon, 16 Dec 2019 10:42:03 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r355611 - head/sys/kern Message-ID: <4e5b7351-d68d-37c9-ed9a-573262a9864e@FreeBSD.org> In-Reply-To: <20191211220028.GW2744@kib.kiev.ua> References: <201912111552.xBBFqUq9009116@repo.freebsd.org> <20191211220028.GW2744@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/12/2019 00:00, Konstantin Belousov wrote: > On Wed, Dec 11, 2019 at 03:52:30PM +0000, Andriy Gapon wrote: >> Author: avg >> Date: Wed Dec 11 15:52:29 2019 >> New Revision: 355611 >> URL: https://svnweb.freebsd.org/changeset/base/355611 >> >> Log: >> add a sanity check to the system call registration code >> >> A system call number should be at least reserved. >> We do not expect an attempt to register a fixed number system call >> when nothing at all is known about it. >> >> MFC after: 3 weeks >> Sponsored by: Panzura >> >> Modified: >> head/sys/kern/kern_syscalls.c >> >> Modified: head/sys/kern/kern_syscalls.c >> ============================================================================== >> --- head/sys/kern/kern_syscalls.c Wed Dec 11 15:15:21 2019 (r355610) >> +++ head/sys/kern/kern_syscalls.c Wed Dec 11 15:52:29 2019 (r355611) >> @@ -120,11 +120,14 @@ kern_syscall_register(struct sysent *sysents, int *off >> if (i == SYS_MAXSYSCALL) >> return (ENFILE); >> *offset = i; >> - } else if (*offset < 0 || *offset >= SYS_MAXSYSCALL) >> + } else if (*offset < 0 || *offset >= SYS_MAXSYSCALL) { >> return (EINVAL); >> - else if (sysents[*offset].sy_call != (sy_call_t *)lkmnosys && >> - sysents[*offset].sy_call != (sy_call_t *)lkmressys) >> + } else if (sysents[*offset].sy_call != (sy_call_t *)lkmnosys && >> + sysents[*offset].sy_call != (sy_call_t *)lkmressys) { >> + KASSERT(sysents[*offset].sy_call != NULL, >> + ("undefined syscall %d", *offset)); > I do not think that the assert is very useful. On debug kernels, where > the development would most likely take place, its only function is to annoy > developer by requiring him to reboot the host if he did not guessed a > free syscall number right. > > I suggest to convert this into printf, might be also under bootverbose. > > Also, why do you say that the syscall is undefined ? I would state that > it is not reusable for dynamically loaded syscall, instead. I admit that panic() could be an overreaction to a problem that I somehow created for myself. And the check is not very robust too (but without false positives) . But we should never see sy_call == NULL in sysents, NULL is not a valid value for the field. I think that somehow my init_sysent.c was out-of-sync with the rest of files generated from syscalls.master. So, I got a situation where I tried to dynamically register a system that was below SYS_MAXSYSCALL but beyond the end of sysent[], and apparently that region happened to be zero-filled. So, confusingly, kern_syscall_register() returned EEXIST. >> return (EEXIST); >> + } >> >> KASSERT(sysents[*offset].sy_thrcnt == SY_THR_ABSENT, >> ("dynamic syscall is not protected")); -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4e5b7351-d68d-37c9-ed9a-573262a9864e>