Re: [Powertop] Seg Fault due to function format_watts in file src/lib.cpp

Re: [Powertop] Seg Fault due to function format_watts in file src/lib.cpp

[Sergey Senozhatsky]

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

On (05/21/13 12:20), Ganapati Bhat wrote:
>    Dear Sergey,
>    Powertop --html is working fine with the patch applied. I am able to
>    successfully execute the command and get the results. However, still
>    trying to understand the patch and the difference when I changed the 0 to
>    len in while (mbstowcs(NULL,buffer,0) < len) inside src/lib.cpp file.
>    Now, will try to adopt the latest version of powertop for android.
>    Thanks for all the help and sorry for the delay.
>    Regards,
>    Ganapati Bhat
>

Thanks Ganapati!

Kristen, could you please take a look if we can land it in upcoming
release?

https://lists.01.org/pipermail/powertop/2013-February/000782.html


Reported-and-Tested-by: Ganapati Bhat <life.mytake(a)gmail.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>


	-ss


 
>    On Tue, May 21, 2013 at 11:23 AM, Ganapati Bhat <[1]life.mytake(a)gmail.com>
>    wrote:
> 
>      Hello,
>      I am really sorry. i was involved in some other tasks. Will get back to
>      you in a couple of days.
>      Ganapati Bhat
> 
>      On Mon, May 20, 2013 at 6:56 PM, Sergey Senozhatsky
>      <[2]sergey.senozhatsky(a)gmail.com> wrote:
> 
>        On (02/28/13 14:45), Ganapati Bhat wrote:
>        >    Dear All,
>        >    I was compiling powertop as an Android external module and using
>        the
>        >    option powertop --html. However, there used to be a segmentation
>        fault.
>        >    When i looked into it, I observed that in the function
>        format_watts in
>        >    src/lib.cpp, the last�argument�to mbstows was zero. By changing
>        the
>        >    argument to len, which is itself an argument to the function
>        format_watts,
>        >    powertop worked properly? Does this also happen on an x86
>        machine? ( I am
>        >    unable to verify this due to some problems in my laptop which I
>        have not
>        >    yet resolved.) If not then can someone explain to me why is it
>        happening
>        >    in the case of android?
>        >    Thanks and Regards,
>        >    Ganapati
> 
>        Hello,
> 
>        did you have any chance to test the patch?
> 
>                -ss
> 
>        > _______________________________________________
>        > PowerTop mailing list
>        > [3]PowerTop(a)lists.01.org
>        > [4]https://lists.01.org/mailman/listinfo/powertop
> 
> References
> 
>    Visible links
>    1. mailto:life.mytake(a)gmail.com
>    2. mailto:sergey.senozhatsky(a)gmail.com
>    3. mailto:PowerTop(a)lists.01.org
>    4. https://lists.01.org/mailman/listinfo/powertop

Re: [Powertop] Seg Fault due to function format_watts in file src/lib.cpp

[Sergey Senozhatsky]

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

On (02/28/13 14:45), Ganapati Bhat wrote:
>    Dear All,
>    I was compiling powertop as an Android external module and using the
>    option powertop --html. However, there used to be a segmentation fault.
>    When i looked into it, I observed that in the function format_watts in
>    src/lib.cpp, the last�argument�to mbstows was zero. By changing the
>    argument to len, which is itself an argument to the function format_watts,
>    powertop worked properly? Does this also happen on an x86 machine? ( I am
>    unable to verify this due to some problems in my laptop which I have not
>    yet resolved.) If not then can someone explain to me why is it happening
>    in the case of android?
>    Thanks and Regards,
>    Ganapati

Hello,

did you have any chance to test the patch?


	-ss

> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop

Re: [Powertop] Seg Fault due to function format_watts in file src/lib.cpp

[Sergey Senozhatsky]

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

On (02/28/13 14:45), Ganapati Bhat wrote:
> Date: Thu, 28 Feb 2013 14:45:04 +0530
> From: Ganapati Bhat <life.mytake(a)gmail.com>
> To: powertop(a)lists.01.org
> Subject: [Powertop] Seg Fault due to function format_watts in file
>  src/lib.cpp
> 
>    Dear All,
>    I was compiling powertop as an Android external module and using the
>    option powertop --html. However, there used to be a segmentation fault.
>    When i looked into it, I observed that in the function format_watts in
>    src/lib.cpp, the last�argument�to mbstows was zero. By changing the
>    argument to len, which is itself an argument to the function format_watts,
>    powertop worked properly? Does this also happen on an x86 machine? ( I am
>    unable to verify this due to some problems in my laptop which I have not
>    yet resolved.) If not then can someone explain to me why is it happening
>    in the case of android?
>    Thanks and Regards,
>    Ganapati

could you try the following patch?

------8<-------8<---------


Use

size_t mbsrtowcs (wchar t *restrict dst, const char **restrict src,
        size t len, mbstate t *restrict ps)

which explicitly handles dst == NULL case:

	If dst is not a null pointer, the result is stored in the array pointed to by dst; otherwise,
	the conversion result is not available since it is stored in an internal buffer.

while

size_t mbstowcs (wchar t *wstring, const char *string, size t size)

does not (at least not in my glibc 2.17 docs), thus is free to cause undefined behavior.

Reported-by: Ganapati Bhat <life.mytake(a)gmail.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>

---

 src/lib.cpp                | 25 +++++++++++++++++++++----
 src/lib.h                  |  1 +
 src/process/do_process.cpp | 10 ++++++----
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/lib.cpp b/src/lib.cpp
index 723517a..1a2f6c8 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -257,22 +257,39 @@ string read_sysfs_string(const char *format, const char *param)
 	return content;
 }
 
+void align_string(char *buffer, size_t min_sz, size_t max_sz)
+{
+	size_t sz;
+
+	/** mbsrtowcs() allows NULL dst and zero sz,
+	 * comparing to mbstowcs(), which causes undefined
+	 * behaviour under given circumstances*/
+
+	/* start with mbsrtowcs() local mbstate_t * and
+	 * NULL dst pointer*/
+	sz = mbsrtowcs(NULL, (const char **)&buffer, max_sz, NULL);
+	if (sz == (size_t)-1) {
+		buffer[min_sz] = 0x00;
+		return;
+	}
+	while (sz < min_sz) {
+		strcat(buffer, " ");
+		sz++;
+	}
+}
 
 void format_watts(double W, char *buffer, unsigned int len)
 {
 	buffer[0] = 0;
 	char buf[32];
-
 	sprintf(buffer, _("%7sW"), fmt_prefix(W, buf));
 
 	if (W < 0.0001)
 		sprintf(buffer, _("    0 mW"));
 
-	while (mbstowcs(NULL,buffer,0) < len)
-		strcat(buffer, " ");
+	align_string(buffer, len, len);
 }
 
-
 #ifndef HAVE_NO_PCI
 static struct pci_access *pci_access;
 
diff --git a/src/lib.h b/src/lib.h
index de2de5a..867f480 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -75,4 +75,5 @@ extern void process_directory(const char *d_name, callback fn);
 extern int utf_ok;
 extern int get_user_input(char *buf, unsigned sz);
 
+extern void align_string(char *buffer, size_t min_sz, size_t max_sz);
 #endif
diff --git a/src/process/do_process.cpp b/src/process/do_process.cpp
index 322ad4c..4aa1a92 100644
--- a/src/process/do_process.cpp
+++ b/src/process/do_process.cpp
@@ -852,13 +852,13 @@ void process_update_display(void)
 		char usage[20];
 		char events[20];
 		char descr[128];
-		format_watts(all_power[i]->Witts(), power, 10);
 
+		format_watts(all_power[i]->Witts(), power, 10);
 		if (!show_power)
 			strcpy(power, "          ");
 		sprintf(name, "%s", all_power[i]->type());
-		while (mbstowcs(NULL,name,0) < 14) strcat(name, " ");
 
+		align_string(name, 14, 20);
 
 		if (all_power[i]->events() == 0 && all_power[i]->usage() == 0 && all_power[i]->Witts() == 0)
 			break;
@@ -870,14 +870,16 @@ void process_update_display(void)
 			else
 				sprintf(usage, "%5i%s", (int)all_power[i]->usage(), all_power[i]->usage_units());
 		}
-		while (mbstowcs(NULL,usage,0) < 14) strcat(usage, " ");
+
+		align_string(usage, 14, 20);
+
 		sprintf(events, "%5.1f", all_power[i]->events());
 		if (!all_power[i]->show_events())
 			events[0] = 0;
 		else if (all_power[i]->events() <= 0.3)
 			sprintf(events, "%5.2f", all_power[i]->events());
 
-		while (strlen(events) < 12) strcat(events, " ");
+		align_string(events, 12, 20);
 		wprintw(win, "%s  %s %s %s %s\n", power, usage, events, name, pretty_print(all_power[i]->description(), descr, 128));
 	}
 }

[Powertop] Seg Fault due to function format_watts in file src/lib.cpp

[Ganapati Bhat]

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

Dear All,

I was compiling powertop as an Android external module and using the option
powertop --html. However, there used to be a segmentation fault. When i
looked into it, I observed that in the function format_watts in
src/lib.cpp, the last argument to mbstows was zero. By changing the
argument to len, which is itself an argument to the function format_watts,
powertop worked properly? Does this also happen on an x86 machine? ( I am
unable to verify this due to some problems in my laptop which I have not
yet resolved.) If not then can someone explain to me why is it happening in
the case of android?

Thanks and Regards,

Ganapati

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 754 bytes --]