Re: [Xenomai-help] Segmentation fault in rt_printf print thread

[Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>]

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Christoph Permes wrote:
>>>> Hi, 
>>>>
>>>> I've created a patch that provides a workaround for the segmentation
>>>> fault:
>>>>
>>>> --- a/src/rtdk/rt_print.c	2009-05-21 16:34:54.000000000 +0200
>>>> +++ b/src/rtdk/rt_print.c	2009-09-04 10:09:19.000000000 +0200
>>>> @@ -40,6 +40,7 @@
>>>>  struct entry_head {
>>>>  	FILE *dest;
>>>>  	uint32_t seq_no;
>>>> +	uint32_t len;
>>>>  	char text[1];
>>>>  } __attribute__((packed));
>>>>  
>>>> @@ -113,6 +114,7 @@
>>>>  			/* Write out empty entry */
>>>>  			head = buffer->ring + write_pos;
>>>>  			head->seq_no = __seq_no;
>>>> +			head->len = 0;
>>>>  			head->text[0] = 0;
>>>>  
>>>>  			/* Forward to the ring buffer start */
>>>> @@ -146,6 +148,7 @@
>>>>  	/* If we were able to write some text, finalise the entry */
>>>>  	if (len > 0) {
>>>>  		head->seq_no = ++__seq_no;
>>>> +		head->len = len;
>>>>  		head->dest = stream;
>>>>  
>>>>  		/* Move forward by text and head length */
>>>> @@ -158,6 +161,7 @@
>>>>  		/* An empty entry marks the wrap-around */
>>>>  		head = buffer->ring + write_pos;
>>>>  		head->seq_no = __seq_no;
>>>> +		head->len = 0;
>>>>  		head->text[0] = 0;
>>>>  
>>>>  		write_pos = 0;
>>>> @@ -373,7 +377,7 @@
>>>>  
>>>>  		read_pos = buffer->read_pos;
>>>>  		head = buffer->ring + read_pos;
>>>> -		len = strlen(head->text);
>>>> +		len = head->len;
>>>>  
>>>>  		if (len) {
>>>>  			/* Print out non-empty entry and proceed */
>>>>
>>>> With this patch I get no segfaults anymore, but of course it would be
>>>> interesting why parts of the ring buffer are overwritten with null
>>>> characters.
>>>>
>>> Frankly, I'm suspecting some issue in your application here - at least
>>> as long as you do not have some broken-out test case for us...
>> Would not it be possible to mprotect the rtdk buffer when it is not used
>> in order to catch any write to it outside of rtdk functions?
>>
> 
> Hmm, good idea, should work. Once set up, only rt_vsnprintf requires
> write access to the ring.

Ok. Here it comes. Note however that calling rt_printf with this patch
will cause the caller to switch to secondary mode, but that is OK for
debugging. Also note that I only compile-tested the patch.

-- 
                                          Gilles


[-- Attachment #2: xeno-rtdk-mprotect.diff --]
[-- Type: text/x-patch, Size: 1476 bytes --]

diff --git a/src/rtdk/rt_print.c b/src/rtdk/rt_print.c
index 10285c0..5e7cda6 100644
--- a/src/rtdk/rt_print.c
+++ b/src/rtdk/rt_print.c
@@ -101,6 +101,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 	read_pos = buffer->read_pos;
 	xnarch_read_memory_barrier();
 
+	mprotect(buffer->ring, buffer->size, PROT_READ | PROT_WRITE);
 	/* Is our write limit the end of the ring buffer? */
 	if (write_pos >= read_pos) {
 		/* Keep a savety margin to the end for at least an empty entry */
@@ -165,6 +166,7 @@ int rt_vfprintf(FILE *stream, const char *format, va_list args)
 
 	/* All entry data must be written before we can update write_pos */
 	xnarch_write_memory_barrier();
+	mprotect(buffer->ring, buffer->size, PROT_READ);
 
 	buffer->write_pos = write_pos;
 
@@ -221,6 +223,7 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 		size = __default_buffer_size;
 	else if (size < RT_PRINT_LINE_BREAK)
 		return EINVAL;
+	size = (size + getpagesize() - 1) & ~(getpagesize() - 1);
 
 	if (buffer) {
 		/* Only set name if buffer size is unchanged or default */
@@ -235,12 +238,11 @@ int rt_print_init(size_t buffer_size, const char *buffer_name)
 	if (!buffer)
 		return ENOMEM;
 
-	buffer->ring = malloc(size);
+	buffer->ring = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE,  -1, 0);
 	if (!buffer->ring) {
 		free(buffer);
 		return ENOMEM;
 	}
-	memset(buffer->ring, 0, size);
 
 	buffer->read_pos  = 0;
 	buffer->write_pos = 0;