svn commit: r355611 - head/sys/kern

Andriy Gapon avg at FreeBSD.org
Mon Dec 16 08:42:08 UTC 2019


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


More information about the svn-src-all mailing list