[PATCH rtems-libbsd] ipsec-tools: Fix copying fd_set prior to select

[Christian MAUDERER]

Hello Chris,

On 2023-05-23 03:36, Chris Johns wrote:
> Hi,
> 
> I have been resolving this by adding:
> 
> #define preset_mask *preset_mask_prealloc
> #define active_mask *active_mask_prealloc
> 
> as the assignment logic works as is. It does not catch FD_ZERO which is a shame
> but is helps avoid us adding a special case.

Thanks for the suggestion. I used that solution already in the file 
during the initial port. But it doesn't work in this case (and maybe we 
should check other cases in libbsd too). It only copies the first 
element of the array of fd_sets.

The variables / defines are (in the patched version - see [1]):

static fd_set *allocated_preset_mask, *allocated_active_mask;
static struct fd_monitor *allocated_fd_monitors;
#define preset_mask (*allocated_preset_mask)
#define active_mask (*allocated_active_mask)
#define fd_monitors (allocated_fd_monitors)

They are allocated as follows (see [2]):

	allocated_preset_mask = calloc(sizeof(fd_set),
	    howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
	allocated_active_mask = calloc(sizeof(fd_set),
	    howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
	allocated_fd_monitors = calloc(
	    rtems_libio_number_iops, sizeof(struct fd_monitor));

So basically it's a bunch of arrays of fd_sets. If I now use the 
following (unpatched) assignment:

	active_mask = preset_mask;

After the preprocessor the line will be

	(*allocated_active_mask) = (*allocated_preset_mask);

Which copies only the first element. That's why I had to add the memcpy. 
Did I miss some better solution?

Best regards

Christian

[1] 
https://git.rtems.org/rtems-libbsd/tree/ipsec-tools/src/racoon/session.c?id=16be3a7c7d3141018c48d5131a3069184cd3937a#n136
[2] 
https://git.rtems.org/rtems-libbsd/tree/ipsec-tools/src/racoon/session.c?id=16be3a7c7d3141018c48d5131a3069184cd3937a#n218

> 
> Chris
> 
> On 22/5/2023 5:36 pm, Christian Mauderer wrote:
>> The racoon session code copies an fd_set from one variable into another
>> prior to calling select. That works well for simple structures.
>>
>> In libbsd we have to allocate fd_sets instead of using fixed structures
>> to avoid a problem with file numbers bigger than FD_SETSIZE. The simple
>> assignment didn't work in that case.
>>
>> This patch makes sure that a memcpy is used instead.
>> ---
>>   ipsec-tools/src/racoon/session.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/ipsec-tools/src/racoon/session.c b/ipsec-tools/src/racoon/session.c
>> index 7ea857ba..bd2bd316 100644
>> --- a/ipsec-tools/src/racoon/session.c
>> +++ b/ipsec-tools/src/racoon/session.c
>> @@ -215,6 +215,8 @@ session(void)
>>   #ifndef __rtems__
>>   	FD_ZERO(&preset_mask);
>>   #else /* __rtems__ */
>> +	size_t allocated_mask_size = sizeof(fd_set) *
>> +	    howmany(rtems_libio_number_iops, sizeof(fd_set) * 8);
>>   	allocated_preset_mask = calloc(sizeof(fd_set),
>>   	    howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
>>   	if (allocated_preset_mask == NULL)
>> @@ -352,7 +354,12 @@ session(void)
>>   
>>   		/* schedular can change select() mask, so we reset
>>   		 * the working copy here */
>> +#ifndef __rtems__
>>   		active_mask = preset_mask;
>> +#else /* __rtems__ */
>> +		memcpy(allocated_active_mask, allocated_preset_mask,
>> +		    allocated_mask_size);
>> +#endif /* __rtems__ */
>>   
>>   		error = select(nfds + 1, &active_mask, NULL, NULL, timeout);
>>   		if (error < 0) {

-- 
--------------------------------------------
embedded brains GmbH & Co. KG
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email:  christian.mauderer at embedded-brains.de
phone:  +49-89-18 94 741 - 18
mobile: +49-176-152 206 08

Registergericht: Amtsgericht München
Registernummer: HRA 117265
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/