2020-07-31 11:45:54

by Wangshaobo (bobo)

Subject: [PATCH v2 -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c

Function jvmti_write_code called by compiled_method_load_cb may return
error in using fwrite_unlocked, this failure should be captured and
warned.

Signed-off-by: Wang ShaoBo <[email protected]>
---
tools/perf/jvmti/jvmti_agent.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
index 88108598d6e9..dfb6cb8564cb 100644
--- a/tools/perf/jvmti/jvmti_agent.c
+++ b/tools/perf/jvmti/jvmti_agent.c
@@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym,
struct jr_code_load rec;
size_t sym_len;
FILE *fp = agent;
- int ret = -1;
+ int sret;

/* don't care about 0 length function, no samples */
if (size == 0)
@@ -400,17 +400,24 @@ jvmti_write_code(void *agent, char const *sym,
*/
rec.code_index = code_generation++;

- ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp);
- fwrite_unlocked(sym, sym_len, 1, fp);
+ sret = fwrite_unlocked(&rec, sizeof(rec), 1, fp);
+ if (sret != 1)
+ goto error;
+ sret = fwrite_unlocked(sym, sym_len, 1, fp);
+ if (sret != 1)
+ goto error;

- if (code)
- fwrite_unlocked(code, size, 1, fp);
+ if (code) {
+ sret = fwrite_unlocked(code, size, 1, fp);
+ if (sret != 1)
+ goto error;
+ }

funlockfile(fp);
-
- ret = 0;
-
- return ret;
+ return 0;
+error:
+ funlockfile(fp);
+ return -1;
}

int
--
2.25.1

2020-08-03 15:43:04

by Ian Rogers

Subject: Re: [PATCH v2 -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c

On Fri, Jul 31, 2020 at 4:42 AM Wang ShaoBo <[email protected]> wrote:
>
> Function jvmti_write_code called by compiled_method_load_cb may return
> error in using fwrite_unlocked, this failure should be captured and
> warned.
>
> Signed-off-by: Wang ShaoBo <[email protected]>

Thanks, looks good now!

Reviewed-by: Ian Rogers <[email protected]>

> ---
> tools/perf/jvmti/jvmti_agent.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
> index 88108598d6e9..dfb6cb8564cb 100644
> --- a/tools/perf/jvmti/jvmti_agent.c
> +++ b/tools/perf/jvmti/jvmti_agent.c
> @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym,
> struct jr_code_load rec;
> size_t sym_len;
> FILE *fp = agent;
> - int ret = -1;
> + int sret;
>
> /* don't care about 0 length function, no samples */
> if (size == 0)
> @@ -400,17 +400,24 @@ jvmti_write_code(void *agent, char const *sym,
> */
> rec.code_index = code_generation++;
>
> - ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp);
> - fwrite_unlocked(sym, sym_len, 1, fp);
> + sret = fwrite_unlocked(&rec, sizeof(rec), 1, fp);
> + if (sret != 1)
> + goto error;
> + sret = fwrite_unlocked(sym, sym_len, 1, fp);
> + if (sret != 1)
> + goto error;
>
> - if (code)
> - fwrite_unlocked(code, size, 1, fp);
> + if (code) {
> + sret = fwrite_unlocked(code, size, 1, fp);
> + if (sret != 1)
> + goto error;
> + }
>
> funlockfile(fp);
> -
> - ret = 0;
> -
> - return ret;
> + return 0;
> +error:
> + funlockfile(fp);
> + return -1;
> }
>
> int
> --
> 2.25.1
>