Fix coverity issues introduced via drm proc module (#15765)
Dimitris P committed
Aug 8, 2023 at 18:52 UTC
56289bdc901c84f62139ec012309cf0ce9a3f9ca
2 files changed
+49
-33
collectors/proc.plugin/sys_class_drm.c
+41
-31
@@ -7,8 +7,8 @@
7
#define AMDGPU_CHART_TYPE "amdgpu"
8
9
struct amdgpu_id_struct {
10
- uint32_t asic_id;
11
- uint32_t pci_rev_id;
10
+ unsigned long long asic_id;
11
+ unsigned long long pci_rev_id;
12
const char *marketing_name;
13
};
14
@@ -27,7 +27,7 @@ struct amdgpu_id_struct {
27
* **IMPORTANT**: The amdgpu_ids has to be modified after new GPU releases.
28
* ------------------------------------------------------------------------*/
29
30
-static struct amdgpu_id_struct amdgpu_ids[] = {
30
+static const struct amdgpu_id_struct amdgpu_ids[] = {
31
{0x1309, 0x00, "AMD Radeon R7 Graphics"},
32
{0x130A, 0x00, "AMD Radeon R6 Graphics"},
33
{0x130B, 0x00, "AMD Radeon R4 Graphics"},
@@ -615,34 +615,46 @@ static void card_free(struct card *c){
615
}
616
617
static int check_card_is_amdgpu(const char *const pathname){
618
- procfile *const ff = procfile_open(pathname, " ", PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
619
- if(unlikely(!procfile_readall(ff))){
620
- procfile_close(ff);
621
- return 1; // error
618
+ int rc = -1;
619
+
620
+ procfile *ff = procfile_open(pathname, " ", PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
621
+ if(unlikely(!ff)){
622
+ rc = -1;
623
+ goto cleanup;
624
+ }
625
+
626
+ ff = procfile_readall(ff);
627
+ if(unlikely(!ff || procfile_lines(ff) < 1 || procfile_linewords(ff, 0) < 1)){
628
+ rc = -2;
629
+ goto cleanup;
630
}
631
632
for(size_t l = 0; l < procfile_lines(ff); l++) {
633
if(!strcmp(procfile_lineword(ff, l, 0), "DRIVER=amdgpu")){
626
- procfile_close(ff);
627
- return 0;
634
+ rc = 0;
635
+ goto cleanup;
636
}
637
}
638
639
+ rc = -3; // no match
640
+
641
+cleanup:
642
procfile_close(ff);
632
- return 2; // no match
643
+ return rc;
644
}
645
635
-static int read_multiline_file(procfile *ff, const char *const pathname, collected_number *num){
636
- if(!ff) ff = procfile_open(pathname, NULL, PROCFILE_FLAG_DEFAULT);
637
- if(unlikely(!procfile_readall(ff))){
638
- procfile_close(ff);
639
- return 1; // error
646
+static int read_clk_freq_file(procfile **p_ff, const char *const pathname, collected_number *num){
647
+ if(unlikely(!*p_ff)){
648
+ *p_ff = procfile_open(pathname, NULL, PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
649
+ if(unlikely(!*p_ff)) return -2;
650
}
651
+
652
+ if(unlikely(NULL == (*p_ff = procfile_readall(*p_ff)))) return -3;
653
642
- for(size_t l = 0; l < procfile_lines(ff) ; l++) {
654
+ for(size_t l = 0; l < procfile_lines(*p_ff) ; l++) {
655
644
- if(ff->lines->lines[l].words >= 3 && !strcmp(procfile_lineword(ff, l, 2), "*")){
645
- char *str_with_units = procfile_lineword(ff, l, 1);
656
+ if((*p_ff)->lines->lines[l].words >= 3 && !strcmp(procfile_lineword((*p_ff), l, 2), "*")){
657
+ char *str_with_units = procfile_lineword((*p_ff), l, 1);
658
char *delim = strchr(str_with_units, 'M');
659
char str_without_units[10];
660
memcpy(str_without_units, str_with_units, delim - str_with_units);
@@ -651,8 +663,8 @@ static int read_multiline_file(procfile *ff, const char *const pathname, collect
663
}
664
}
665
654
- procfile_close(ff);
655
- return 2; // error
666
+ procfile_close((*p_ff));
667
+ return -4;
668
}
669
670
static char *set_id(const char *const suf_1, const char *const suf_2, const char *const suf_3){
@@ -715,7 +727,7 @@ static int do_rrd_util_mem(struct card *const c){
727
}
728
729
static int do_rrd_clk_gpu(struct card *const c){
718
- if(likely(!read_multiline_file(c->ff_clk_gpu, (char *) c->pathname_clk_gpu, &c->clk_gpu))){
730
+ if(likely(!read_clk_freq_file(&c->ff_clk_gpu, (char *) c->pathname_clk_gpu, &c->clk_gpu))){
731
rrddim_set_by_pointer(c->st_clk_gpu, c->rd_clk_gpu, c->clk_gpu);
732
rrdset_done(c->st_clk_gpu);
733
return 0;
@@ -724,13 +736,12 @@ static int do_rrd_clk_gpu(struct card *const c){
736
collector_error("Cannot read clk_gpu for %s: [%s]", c->pathname, c->id.marketing_name);
737
freez((void *) c->pathname_clk_gpu);
738
rrdset_is_obsolete(c->st_clk_gpu);
727
- procfile_close(c->ff_clk_gpu);
739
return 1;
740
}
741
}
742
743
static int do_rrd_clk_mem(struct card *const c){
733
- if(likely(!read_multiline_file(c->ff_clk_mem, (char *) c->pathname_clk_mem, &c->clk_mem))){
744
+ if(likely(!read_clk_freq_file(&c->ff_clk_mem, (char *) c->pathname_clk_mem, &c->clk_mem))){
745
rrddim_set_by_pointer(c->st_clk_mem, c->rd_clk_mem, c->clk_mem);
746
rrdset_done(c->st_clk_mem);
747
return 0;
@@ -739,7 +750,6 @@ static int do_rrd_clk_mem(struct card *const c){
750
collector_error("Cannot read clk_mem for %s: [%s]", c->pathname, c->id.marketing_name);
751
freez((void *) c->pathname_clk_mem);
752
rrdset_is_obsolete(c->st_clk_mem);
742
- procfile_close(c->ff_clk_mem);
753
return 1;
754
}
755
}
@@ -847,14 +857,14 @@ int do_sys_class_drm(int update_every, usec_t dt) {
857
c->pathname = strdupz(filename);
858
859
snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/device");
850
- if(read_single_base64_or_hex_number_file(filename, (unsigned long long *) &c->id.asic_id)){
860
+ if(read_single_base64_or_hex_number_file(filename, &c->id.asic_id)){
861
collector_error("Cannot read asic_id from '%s'", filename);
862
card_free(c);
863
continue;
864
}
865
866
snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/revision");
857
- if(read_single_base64_or_hex_number_file(filename, (unsigned long long *) &c->id.pci_rev_id)){
867
+ if(read_single_base64_or_hex_number_file(filename, &c->id.pci_rev_id)){
868
collector_error("Cannot read pci_rev_id from '%s'", filename);
869
card_free(c);
870
continue;
@@ -871,13 +881,13 @@ int do_sys_class_drm(int update_every, usec_t dt) {
881
882
883
collected_number tmp_val;
874
- #define set_prop_pathname(prop_filename, prop_pathname, ff){ \
884
+ #define set_prop_pathname(prop_filename, prop_pathname, p_ff){ \
885
snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, prop_filename); \
876
- if((ff && !read_multiline_file(ff, filename, &tmp_val)) || \
886
+ if((p_ff && !read_clk_freq_file(p_ff, filename, &tmp_val)) || \
887
!read_single_number_file(filename, (unsigned long long *) &tmp_val)) \
888
prop_pathname = strdupz(filename); \
889
else \
880
- collector_info("Cannot read file '%s'", filename); \
890
+ collector_info("Cannot read file '%s'", filename); \
891
}
892
893
/* Initialize GPU and VRAM utilization metrics */
@@ -935,7 +945,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
945
946
/* Initialize GPU and VRAM clock frequency metrics */
947
938
- set_prop_pathname("device/pp_dpm_sclk", c->pathname_clk_gpu, c->ff_clk_gpu);
948
+ set_prop_pathname("device/pp_dpm_sclk", c->pathname_clk_gpu, &c->ff_clk_gpu);
949
950
if(c->pathname_clk_gpu){
951
c->st_clk_gpu = rrdset_create_localhost(
@@ -961,7 +971,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
971
972
}
973
964
- set_prop_pathname("device/pp_dpm_mclk", c->pathname_clk_mem, c->ff_clk_mem);
974
+ set_prop_pathname("device/pp_dpm_mclk", c->pathname_clk_mem, &c->ff_clk_mem);
975
976
if(c->pathname_clk_mem){
977
c->st_clk_mem = rrdset_create_localhost(
libnetdata/inlined.h
+8
-2
@@ -521,8 +521,14 @@ static inline int read_single_base64_or_hex_number_file(const char *filename, un
521
522
buffer[30] = '\0';
523
524
- *result = str2ull_encoded(buffer);
525
- return 0;
524
+ if(likely(buffer[0])){
525
+ *result = str2ull_encoded(buffer);
526
+ return 0;
527
+ }
528
+ else {
529
+ *result = 0;
530
+ return -1;
531
+ }
532
}
533
534
static inline int uuid_memcmp(const uuid_t *uu1, const uuid_t *uu2) {