Skip to content

Commit 1f1ee3c

Browse files
PiloalucardPiloalucard
authored
fix: Memory leaks (#575)
1 parent e833b0d commit 1f1ee3c

File tree

8 files changed

+164
-107
lines changed

8 files changed

+164
-107
lines changed

‎src/libltfs/arch/ltfs_arch_ops.h‎

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
** OO_Copyright_BEGIN
33
**
44
**
5-
** Copyright 2025 IBM Corp. All rights reserved.
5+
** Copyright 2026 IBM Corp. All rights reserved.
66
**
77
** Redistribution and use in source and binary forms, with or without
88
** modification, are permitted provided that the following conditions
@@ -74,16 +74,6 @@ extern "C" {
7474
} \
7575
}while(0)
7676

77-
static inline void arch_strcpy_limited(char *dest, const char *src, int count)
78-
{
79-
int i;
80-
for (i = 0; i < (count) && (src)[i] != '\0'; i++)
81-
(dest)[i] = (src)[i];
82-
if (i < (count))
83-
(dest)[i] = '\0';
84-
}
85-
86-
8777

8878
#ifdef _MSC_VER
8979
#include <libxml/xmlmemory.h>
@@ -114,18 +104,18 @@ static inline void arch_strcpy_limited(char *dest, const char *src, int count)
114104

115105
#define arch_fopen(file, mode, file_ptr) fopen_s(&(file_ptr), file, mode)
116106

117-
#define arch_ctime(buf, time_ptr) ctime_s(buf, sizeof(buf), time_ptr)
118-
119107
#define arch_getenv(buf, name) do { size_t len; _dupenv_s(&(buf), &(len), name); } while (0)
120108

121-
#define arch_strtok(str, delm, ctxt) strtok_s((str), (delm), &(ctxt))
122-
123109
#define arch_strcpy(dest, size, src) strcpy_s((dest), (size), (src))
124110

125111
#define arch_strncpy(dest, src, size, cnt) strncpy_s((dest), (size), (src), (cnt))
126112

127113
#define arch_strcat(dest, size, src) strcat_s((dest), (size), (src))
128114

115+
#define arch_strtok(str, delm, ctxt) strtok_s((str), (delm), &(ctxt))
116+
117+
#define arch_ctime(buf, time_ptr) ctime_s(buf, sizeof(buf), time_ptr)
118+
129119
#define arch_unlink _unlink
130120

131121
#define arch_write _write
@@ -165,18 +155,18 @@ static inline void arch_strcpy_limited(char *dest, const char *src, int count)
165155

166156
#define arch_fopen(file, mode, file_ptr) do {file_ptr = fopen(file, mode);}while(0)
167157

168-
#define arch_ctime(buf ,time_ptr) do { buf = ctime(time_ptr); } while (0)
169-
170158
#define arch_getenv(buf ,name) do { buf = getenv(name); } while (0)
171159

172-
#define arch_strcpy(dest, unused, src) ({if(unused || !unused) {strcpy(dest, src);}})
160+
#define arch_strcpy(dest, unused, src) ((void)(unused), strcpy(dest, src))
173161

174-
#define arch_strncpy(dest, src, unused, cnt) strncpy(dest, src, cnt)
162+
#define arch_strncpy(dest, src, destSize, cnt) strncpy(dest, src, (cnt))
175163

176-
#define arch_strcat(dest, unused, src)( {if(unused || !unused){ strcat(dest, src);}})
164+
#define arch_strcat(dest, unused, src) ((void)(unused), strcat(dest, src))
177165

178166
#define arch_strtok(str, delim, unused) ((void)(unused), strtok(str, delim))
179167

168+
#define arch_ctime(buf ,time_ptr) do { buf = ctime(time_ptr); } while (0)
169+
180170
#define arch_unlink unlink
181171

182172
#define arch_write write
@@ -198,14 +188,15 @@ static inline void arch_strcpy_limited(char *dest, const char *src, int count)
198188

199189
#endif /* _MSC_VER */
200190

201-
/* These needs to be declared at the end to avoid redefinition and to avoid code replication */
202-
#define arch_vsprintf_auto( buffer, fmt, ...) arch_vsprintf(buffer,sizeof(buffer),fmt,__VA_ARGS__)
203-
191+
/*
192+
These macros need to be declared at the end to avoid redefinition and to avoid code replication
193+
When using them, dest or buffer needs to be a fixed size array since it will calculate it
194+
with the sizeof.
195+
*/
196+
204197
#define arch_strcpy_auto(dest, src) arch_strcpy(dest, sizeof(dest), src);
205198

206-
#define arch_strncpy_auto(dest, src, destSize) arch_strncpy(dest, src, destSize, destSize);
207-
208-
#define arch_strcat_auto(dest,src) arch_strcat(dest, sizeof(dest), src);
199+
#define arch_strncpy_auto(dest, src, count) arch_strncpy(dest, src, sizeof(dest), count);
209200

210201
#define arch_sprintf_auto(buffer, fmt, ...) arch_sprintf(buffer,sizeof(buffer),fmt, __VA_ARGS__)
211202

‎src/libltfs/fs.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ int fs_hash_sort_by_uid(struct name_list *a, struct name_list *b)
7777
static char* generate_hash_key_name(const char *src_str, int *rc)
7878
{
7979
char *key_name = NULL;
80-
key_name = malloc(sizeof(char*));
81-
if (key_name == NULL) return NULL;
8280
#ifdef mingw_PLATFORM
8381
UChar *uchar_name;
8482

@@ -89,11 +87,16 @@ static char* generate_hash_key_name(const char *src_str, int *rc)
8987

9088
if (*rc != 0) {
9189
key_name = NULL;
92-
} else
93-
free(uchar_name);
90+
} else{
91+
arch_safe_free(uchar_name);
92+
}
9493
#else
9594
key_name = arch_strdup(src_str);
96-
*rc = 0;
95+
if (key_name){
96+
*rc = 0;
97+
}else{
98+
*rc = -LTFS_NO_MEMORY;
99+
}
97100
#endif
98101

99102
return key_name;

‎src/libltfs/ltfs_fsops.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,14 +2063,15 @@ int ltfs_fsops_target_absolute_path(const char* link, const char* target, char*
20632063
len -= strlen(temp_buf); /* length of "/aaa" */
20642064
} else if (strcmp(token, "." )) { /* have directory name */
20652065
work_buf[len] = '/'; /* put '/ 'as "/aaa/" */
2066-
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token) + 1); /* "/aaa/ccc\0" */
2066+
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token) ); /* "/aaa/ccc\0" */
20672067
len = strlen(work_buf);
20682068
}
20692069
token = next_token;
20702070
}
20712071
work_buf[len] = '/'; /* put '/ 'as "/aaa/ccc/" */
2072-
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token)+1); /* "/aaa/ccc/target.txt\0" */
2073-
2072+
if(token){
2073+
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token)); /* "/aaa/ccc/target.txt\0" */
2074+
}
20742075
if (size < strlen(work_buf) + 1) {
20752076
free(work_buf);
20762077
free(target_buf);

‎src/libltfs/ltfslogging.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ int ltfsmsg_internal(bool print_id, int level, char **msg_out, const char *_id,
396396
goto internal_error;
397397

398398
if (idlen > 1 && _id[0] == '"' && _id[idlen - 1] == '"') {
399-
arch_strcpy_limited(id, _id + 1, idlen - 2);
399+
arch_strncpy_auto(id, _id + 1, idlen - 2);
400400
id[idlen - 2] = '\0';
401401
}
402402
else {

‎src/libltfs/tape.c‎

Lines changed: 99 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1782,7 +1782,8 @@ int tape_set_cart_coherency(struct device_data *dev, const tape_partition_t part
17821782
/* APPLICATION CLIENT SPECIFIC INFORMATION LENGTH */
17831783
coh_data[30] = 0; /* Size of APPLICATION CLIENT SPECIFIC INFORMATION (Byte 1) */
17841784
coh_data[31] = 43; /* Size of APPLICATION CLIENT SPECIFIC INFORMATION (Byte 0) */
1785-
arch_strcpy_auto((char *)coh_data + 32, "LTFS");
1785+
/* Size of the buffer to insert 'LTFS' needs to be size of 5 for the 4 letters and the null terminator*/
1786+
arch_strncpy((char *)coh_data + 32,"LTFS", 5, 4);
17861787
memcpy(coh_data + 37, coh->uuid, 37);
17871788
/*
17881789
Version field
@@ -3063,84 +3064,128 @@ void set_tape_attribute(struct ltfs_volume *vol, struct tape_attr *t_attr)
30633064
*/
30643065
int tape_set_attribute_to_cm(struct device_data *dev, struct tape_attr *t_attr, int type)
30653066
{
3066-
30673067
int ret;
30683068
int attr_size;
30693069
uint8_t format;
3070+
unsigned char *attr_data = NULL;
3071+
unsigned char *data;
3072+
size_t len;
30703073

30713074
CHECK_ARG_NULL(dev, -LTFS_NULL_ARG);
30723075
CHECK_ARG_NULL(t_attr, -LTFS_NULL_ARG);
30733076

3074-
if ( type == TC_MAM_APP_VENDER ) {
3077+
switch (type) {
3078+
case TC_MAM_APP_VENDER:
30753079
attr_size = TC_MAM_APP_VENDER_SIZE;
30763080
format = ASCII_FORMAT;
3077-
} else if ( type == TC_MAM_APP_NAME) {
3081+
break;
3082+
case TC_MAM_APP_NAME:
30783083
attr_size = TC_MAM_APP_NAME_SIZE;
30793084
format = ASCII_FORMAT;
3080-
} else if ( type== TC_MAM_APP_VERSION ) {
3085+
break;
3086+
case TC_MAM_APP_VERSION:
30813087
attr_size = TC_MAM_APP_VERSION_SIZE;
30823088
format = ASCII_FORMAT;
3083-
} else if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
3089+
break;
3090+
case TC_MAM_USER_MEDIUM_LABEL:
30843091
attr_size = TC_MAM_USER_MEDIUM_LABEL_SIZE;
30853092
format = TEXT_FORMAT;
3086-
} else if ( type == TC_MAM_TEXT_LOCALIZATION_IDENTIFIER ) {
3093+
break;
3094+
case TC_MAM_TEXT_LOCALIZATION_IDENTIFIER:
30873095
attr_size = TC_MAM_TEXT_LOCALIZATION_IDENTIFIER_SIZE;
30883096
format = BINARY_FORMAT;
3089-
} else if ( type == TC_MAM_BARCODE ) {
3097+
break;
3098+
case TC_MAM_BARCODE:
30903099
attr_size = TC_MAM_BARCODE_SIZE;
30913100
format = ASCII_FORMAT;
3092-
} else if ( type == TC_MAM_APP_FORMAT_VERSION ) {
3101+
break;
3102+
case TC_MAM_APP_FORMAT_VERSION:
30933103
attr_size = TC_MAM_APP_FORMAT_VERSION_SIZE;
30943104
format = ASCII_FORMAT;
3095-
} else if ( type == TC_MAM_LOCKED_MAM ) {
3105+
break;
3106+
case TC_MAM_LOCKED_MAM:
30963107
attr_size = TC_MAM_LOCKED_MAM_SIZE;
30973108
format = BINARY_FORMAT;
3098-
} else if ( type == TC_MAM_MEDIA_POOL ) {
3109+
break;
3110+
case TC_MAM_MEDIA_POOL:
30993111
attr_size = TC_MAM_MEDIA_POOL_SIZE;
31003112
format = TEXT_FORMAT;
3101-
} else {
3113+
break;
3114+
default:
31023115
ltfsmsg(LTFS_WARN, 17204W, type, "tape_set_attribute_to_cm");
31033116
return -1;
31043117
}
31053118

3106-
unsigned char *attr_data = NULL;
3107-
attr_data = (unsigned char*)malloc(sizeof(unsigned char*)*(attr_size +TC_MAM_PAGE_HEADER_SIZE));
3108-
if (attr_data == NULL) return -LTFS_NO_MEMORY;
3109-
ltfs_u16tobe(attr_data, type); /* set attribute type */
3119+
/* we reserve the size of the attribute + the MAM header size since the buffer will contain both */
3120+
attr_data = calloc(1, attr_size + TC_MAM_PAGE_HEADER_SIZE);
3121+
if (!attr_data) {
3122+
return -LTFS_NO_MEMORY;
3123+
}
3124+
3125+
/* fill the MAM header information */
3126+
ltfs_u16tobe(attr_data, type); /* set attribute type */
31103127
attr_data[2] = format; /* set data format type */
31113128
ltfs_u16tobe(attr_data + 3, attr_size); /* set data size */
31123129

3130+
/* data becomes the remaining space after TC_MAM_PAGE_HEADER_SIZE to start writing in, that is the reason why we add it here to the buffer address */
3131+
data = attr_data + TC_MAM_PAGE_HEADER_SIZE;
3132+
31133133
/* set attribute data */
3114-
if ( type == TC_MAM_APP_VENDER ) {
3115-
arch_strncpy((char *)attr_data + 5, t_attr->vender, attr_size+ TC_MAM_PAGE_HEADER_SIZE , attr_size);
3116-
} else if ( type == TC_MAM_APP_NAME ) {
3117-
arch_strncpy((char *)attr_data + 5, t_attr->app_name, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3118-
} else if ( type == TC_MAM_APP_VERSION ) {
3119-
arch_strncpy((char *)attr_data + 5, t_attr->app_ver, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3120-
} else if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
3121-
arch_strncpy((char *)attr_data + 5, t_attr->medium_label, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3122-
} else if ( type == TC_MAM_TEXT_LOCALIZATION_IDENTIFIER ) {
3123-
attr_data[5] = t_attr->tli;
3124-
} else if ( type == TC_MAM_BARCODE ) {
3125-
arch_strncpy((char *)attr_data + 5, t_attr->barcode, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3126-
} else if ( type == TC_MAM_APP_FORMAT_VERSION ) {
3127-
arch_strncpy((char *)attr_data + 5, t_attr->app_format_ver, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3128-
} else if ( type == TC_MAM_LOCKED_MAM ) {
3129-
attr_data[5] = t_attr->vollock;
3130-
} else if ( type == TC_MAM_MEDIA_POOL) {
3131-
arch_strncpy((char *)attr_data + 5, t_attr->media_pool, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
3134+
switch (type) {
3135+
case TC_MAM_APP_VENDER:
3136+
len = strnlen(t_attr->vender, attr_size);
3137+
memcpy(data, t_attr->vender, len);
3138+
break;
3139+
3140+
case TC_MAM_APP_NAME:
3141+
len = strnlen(t_attr->app_name, attr_size);
3142+
memcpy(data, t_attr->app_name, len);
3143+
break;
3144+
3145+
case TC_MAM_APP_VERSION:
3146+
len = strnlen(t_attr->app_ver, attr_size);
3147+
memcpy(data, t_attr->app_ver, len);
3148+
break;
3149+
3150+
case TC_MAM_USER_MEDIUM_LABEL:
3151+
len = strnlen(t_attr->medium_label, attr_size);
3152+
memcpy(data, t_attr->medium_label, len);
3153+
break;
3154+
3155+
case TC_MAM_TEXT_LOCALIZATION_IDENTIFIER:
3156+
data[0] = t_attr->tli;
3157+
break;
3158+
3159+
case TC_MAM_BARCODE:
3160+
len = strnlen(t_attr->barcode, attr_size);
3161+
memcpy(data, t_attr->barcode, len);
3162+
break;
3163+
3164+
case TC_MAM_APP_FORMAT_VERSION:
3165+
len = strnlen(t_attr->app_format_ver, attr_size);
3166+
memcpy(data, t_attr->app_format_ver, len);
3167+
break;
3168+
3169+
case TC_MAM_LOCKED_MAM:
3170+
data[0] = t_attr->vollock;
3171+
break;
3172+
3173+
case TC_MAM_MEDIA_POOL:
3174+
len = strnlen(t_attr->media_pool, attr_size);
3175+
memcpy(data, t_attr->media_pool, len);
3176+
break;
31323177
}
31333178

31343179
ret = dev->backend->write_attribute(dev->backend_data,
3135-
0, /* partition */
3136-
attr_data,
3137-
(attr_size + TC_MAM_PAGE_HEADER_SIZE));
3180+
0, /* partition */
3181+
attr_data,
3182+
attr_size + TC_MAM_PAGE_HEADER_SIZE);
31383183

3139-
if (ret < 0)
3184+
if (ret < 0) {
31403185
ltfsmsg(LTFS_ERR, 17205E, type, "tape_set_attribute_to_cm");
3186+
}
31413187
free(attr_data);
31423188
return ret;
3143-
31443189
}
31453190

31463191
/**
@@ -3218,12 +3263,13 @@ int tape_get_attribute_from_cm(struct device_data *dev, struct tape_attr *t_attr
32183263
{
32193264
int ret;
32203265
int attr_len;
3266+
unsigned char *attr_data = NULL;
32213267

32223268
CHECK_ARG_NULL(dev, -LTFS_NULL_ARG);
32233269
CHECK_ARG_NULL(t_attr, -LTFS_NULL_ARG);
32243270

32253271
switch (type) {
3226-
case TC_MAM_APP_VENDER:
3272+
case TC_MAM_APP_VENDER:
32273273
attr_len = TC_MAM_APP_VENDER_SIZE;
32283274
break;
32293275
case TC_MAM_APP_NAME:
@@ -3256,14 +3302,16 @@ int tape_get_attribute_from_cm(struct device_data *dev, struct tape_attr *t_attr
32563302
break;
32573303
}
32583304

3259-
unsigned char *attr_data = NULL;
3260-
attr_data = malloc(sizeof(char*) * (attr_len + TC_MAM_PAGE_HEADER_SIZE));
3261-
if (attr_data == NULL) return -LTFS_NO_MEMORY;
3305+
int attr_size = sizeof(char) * (attr_len + TC_MAM_PAGE_HEADER_SIZE);
3306+
attr_data = (unsigned char*)malloc(attr_size);
3307+
if (!attr_data) {
3308+
return -LTFS_NO_MEMORY;
3309+
}
32623310
ret = dev->backend->read_attribute(dev->backend_data,
3263-
0, /* partition */
3264-
type,
3265-
attr_data,
3266-
sizeof(attr_data));
3311+
0, /* partition */
3312+
type,
3313+
attr_data,
3314+
attr_size);
32673315

32683316
if (ret == 0) {
32693317
uint16_t id = ltfs_betou16(attr_data);
@@ -3304,8 +3352,9 @@ int tape_get_attribute_from_cm(struct device_data *dev, struct tape_attr *t_attr
33043352
memcpy(t_attr->media_pool, attr_data + 5, attr_len);
33053353
t_attr->media_pool[attr_len] = '\0';
33063354
}
3307-
} else
3355+
} else {
33083356
ltfsmsg(LTFS_DEBUG, 17198D, type, "tape_get_attribute_from_cm");
3357+
}
33093358
free(attr_data);
33103359
return ret;
33113360
}
@@ -3472,10 +3521,10 @@ int update_tape_attribute(struct ltfs_volume *vol, const char *new_value, int ty
34723521
if (ret < 0) {
34733522
if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
34743523
memset(vol->t_attr->medium_label, '\0', TC_MAM_USER_MEDIUM_LABEL_SIZE + 1);
3475-
arch_strncpy_auto(vol->t_attr->medium_label, pre_attr, strlen(pre_attr));
3524+
arch_strcpy_auto(vol->t_attr->medium_label, pre_attr);
34763525
} else if (type == TC_MAM_BARCODE) {
34773526
memset(vol->t_attr->barcode, '\0', TC_MAM_BARCODE_SIZE + 1);
3478-
arch_strncpy_auto(vol->t_attr->barcode, pre_attr, strlen(pre_attr));
3527+
arch_strcpy_auto(vol->t_attr->barcode, pre_attr);
34793528
}
34803529
}
34813530

0 commit comments

Comments
 (0)