Skip to content

Fix: Memory leaks #575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Piloalucard wants to merge 4 commits into
base: v2.4.8-windows-support
Choose a base branch
from
Open

Fix: Memory leaks #575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fb7303
Fix: Memory leaks
Piloalucard Mar 5, 2026
81e2a9c
Fix formatting of variable declarations in xml_reader_libltfs.c
Piloalucard Mar 7, 2026
be5ec23
fix: changed strcpy for strncpy
Piloalucard Mar 9, 2026
6ef93d4
Clarify data pointer adjustment in tape.c
Piloalucard Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions src/libltfs/arch/ltfs_arch_ops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,6 @@ extern "C" {
} \
}while(0)

static inline void arch_strcpy_limited(char *dest, const char *src, int count)
{
int i;
for (i = 0; i < (count) && (src)[i] != '\0'; i++)
(dest)[i] = (src)[i];
if (i < (count))
(dest)[i] = '\0';
}



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

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

#define arch_ctime(buf, time_ptr) ctime_s(buf, sizeof(buf), time_ptr)

#define arch_getenv(buf, name) do { size_t len; _dupenv_s(&(buf), &(len), name); } while (0)

#define arch_strtok(str, delm, ctxt) strtok_s((str), (delm), &(ctxt))

#define arch_strcpy(dest, size, src) strcpy_s((dest), (size), (src))

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

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

#define arch_strtok(str, delm, ctxt) strtok_s((str), (delm), &(ctxt))

#define arch_ctime(buf, time_ptr) ctime_s(buf, sizeof(buf), time_ptr)

#define arch_unlink _unlink

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

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

#define arch_ctime(buf ,time_ptr) do { buf = ctime(time_ptr); } while (0)

#define arch_getenv(buf ,name) do { buf = getenv(name); } while (0)

#define arch_strcpy(dest, unused, src) ({if(unused || !unused) {strcpy(dest, src);}})
#define arch_strcpy(dest, unused, src) ((void)(unused), strcpy(dest, src))

#define arch_strncpy(dest, src, unused, cnt) strncpy(dest, src, cnt)
#define arch_strncpy(dest, src, destSize, cnt) strncpy(dest, src, (cnt))

#define arch_strcat(dest, unused, src)( {if(unused || !unused){ strcat(dest, src);}})
#define arch_strcat(dest, unused, src) ((void)(unused), strcat(dest, src))

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

#define arch_ctime(buf ,time_ptr) do { buf = ctime(time_ptr); } while (0)

#define arch_unlink unlink

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

#endif /* _MSC_VER */

/* These needs to be declared at the end to avoid redefinition and to avoid code replication */
#define arch_vsprintf_auto( buffer, fmt, ...) arch_vsprintf(buffer,sizeof(buffer),fmt,__VA_ARGS__)

/*
These needs to be declared at the end to avoid redefinition and to avoid code replication
When using them, dest or buffer needs to be a fixed size array since it will calculate it
with the sizeof.
*/

#define arch_strcpy_auto(dest, src) arch_strcpy(dest, sizeof(dest), src);

#define arch_strncpy_auto(dest, src, destSize) arch_strncpy(dest, src, destSize, destSize);

#define arch_strcat_auto(dest,src) arch_strcat(dest, sizeof(dest), src);
#define arch_strncpy_auto(dest, src, count) arch_strncpy(dest, src, sizeof(dest), count);

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

Expand Down
13 changes: 8 additions & 5 deletions src/libltfs/fs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ int fs_hash_sort_by_uid(struct name_list *a, struct name_list *b)
static char* generate_hash_key_name(const char *src_str, int *rc)
{
char *key_name = NULL;
key_name = malloc(sizeof(char*));
if (key_name == NULL) return NULL;
#ifdef mingw_PLATFORM
UChar *uchar_name;

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

if (*rc != 0) {
key_name = NULL;
} else
free(uchar_name);
} else{
arch_safe_free(uchar_name);
}
#else
key_name = arch_strdup(src_str);
*rc = 0;
if (key_name){
*rc = 0;
}else{
*rc = -LTFS_NO_MEMORY;
}
#endif

return key_name;
Expand Down
5 changes: 3 additions & 2 deletions src/libltfs/ltfs_fsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2063,13 +2063,14 @@ int ltfs_fsops_target_absolute_path(const char* link, const char* target, char*
len -= strlen(temp_buf); /* length of "/aaa" */
} else if (strcmp(token, "." )) { /* have directory name */
work_buf[len] = '/'; /* put '/ 'as "/aaa/" */
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token) + 1); /* "/aaa/ccc\0" */
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token) ); /* "/aaa/ccc\0" */
len = strlen(work_buf);
}
token = next_token;
}
work_buf[len] = '/'; /* put '/ 'as "/aaa/ccc/" */
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token)+1); /* "/aaa/ccc/target.txt\0" */
if(token)
arch_strncpy(work_buf+len+1, token, work_buf_len, strlen(token)); /* "/aaa/ccc/target.txt\0" */

if (size < strlen(work_buf) + 1) {
free(work_buf);
Expand Down
2 changes: 1 addition & 1 deletion src/libltfs/ltfslogging.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ int ltfsmsg_internal(bool print_id, int level, char **msg_out, const char *_id,
goto internal_error;

if (idlen > 1 && _id[0] == '"' && _id[idlen - 1] == '"') {
arch_strcpy_limited(id, _id + 1, idlen - 2);
arch_strncpy_auto(id, _id + 1, idlen - 2);
id[idlen - 2] = '\0';
}
else {
Expand Down
142 changes: 93 additions & 49 deletions src/libltfs/tape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,7 +1782,7 @@ int tape_set_cart_coherency(struct device_data *dev, const tape_partition_t part
/* APPLICATION CLIENT SPECIFIC INFORMATION LENGTH */
coh_data[30] = 0; /* Size of APPLICATION CLIENT SPECIFIC INFORMATION (Byte 1) */
coh_data[31] = 43; /* Size of APPLICATION CLIENT SPECIFIC INFORMATION (Byte 0) */
arch_strcpy_auto((char *)coh_data + 32, "LTFS");
arch_strncpy((char *)coh_data + 32,"LTFS", 5, 4);
memcpy(coh_data + 37, coh->uuid, 37);
/*
Version field
Expand Down Expand Up @@ -3061,86 +3061,129 @@ void set_tape_attribute(struct ltfs_volume *vol, struct tape_attr *t_attr)
* @param set attribute type
* @return 0 positive : success, negative : cannot set value to Cartridge Memory
*/
int tape_set_attribute_to_cm(struct device_data *dev, struct tape_attr *t_attr, int type)
int tape_set_attribute_to_cm(struct device_data* dev,
struct tape_attr* t_attr,
int type)
{

int ret;
int attr_size;
uint8_t format;
unsigned char* attr_data = NULL;
unsigned char* data;
size_t len;

CHECK_ARG_NULL(dev, -LTFS_NULL_ARG);
CHECK_ARG_NULL(t_attr, -LTFS_NULL_ARG);

if ( type == TC_MAM_APP_VENDER ) {
switch (type) {
case TC_MAM_APP_VENDER:
attr_size = TC_MAM_APP_VENDER_SIZE;
format = ASCII_FORMAT;
} else if ( type == TC_MAM_APP_NAME) {
break;
case TC_MAM_APP_NAME:
attr_size = TC_MAM_APP_NAME_SIZE;
format = ASCII_FORMAT;
} else if ( type== TC_MAM_APP_VERSION ) {
break;
case TC_MAM_APP_VERSION:
attr_size = TC_MAM_APP_VERSION_SIZE;
format = ASCII_FORMAT;
} else if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
break;
case TC_MAM_USER_MEDIUM_LABEL:
attr_size = TC_MAM_USER_MEDIUM_LABEL_SIZE;
format = TEXT_FORMAT;
} else if ( type == TC_MAM_TEXT_LOCALIZATION_IDENTIFIER ) {
break;
case TC_MAM_TEXT_LOCALIZATION_IDENTIFIER:
attr_size = TC_MAM_TEXT_LOCALIZATION_IDENTIFIER_SIZE;
format = BINARY_FORMAT;
} else if ( type == TC_MAM_BARCODE ) {
break;
case TC_MAM_BARCODE:
attr_size = TC_MAM_BARCODE_SIZE;
format = ASCII_FORMAT;
} else if ( type == TC_MAM_APP_FORMAT_VERSION ) {
break;
case TC_MAM_APP_FORMAT_VERSION:
attr_size = TC_MAM_APP_FORMAT_VERSION_SIZE;
format = ASCII_FORMAT;
} else if ( type == TC_MAM_LOCKED_MAM ) {
break;
case TC_MAM_LOCKED_MAM:
attr_size = TC_MAM_LOCKED_MAM_SIZE;
format = BINARY_FORMAT;
} else if ( type == TC_MAM_MEDIA_POOL ) {
break;
case TC_MAM_MEDIA_POOL:
attr_size = TC_MAM_MEDIA_POOL_SIZE;
format = TEXT_FORMAT;
} else {
break;
default:
ltfsmsg(LTFS_WARN, 17204W, type, "tape_set_attribute_to_cm");
return -1;
}

unsigned char *attr_data = NULL;
attr_data = (unsigned char*)malloc(sizeof(unsigned char*)*(attr_size +TC_MAM_PAGE_HEADER_SIZE));
if (attr_data == NULL) return -LTFS_NO_MEMORY;
ltfs_u16tobe(attr_data, type); /* set attribute type */
attr_data = calloc(1, attr_size + TC_MAM_PAGE_HEADER_SIZE);
if (!attr_data)
return -LTFS_NO_MEMORY;

ltfs_u16tobe(attr_data, type); /* set attribute type */
attr_data[2] = format; /* set data format type */
ltfs_u16tobe(attr_data + 3, attr_size); /* set data size */

/* data becomes the available space after TC_MAM_PAGE_HEADER_SIZE to start writing in , that is the reason why we add it here*/
data = attr_data + TC_MAM_PAGE_HEADER_SIZE;
Copy link
Contributor

@amissael95 amissael95 Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that what you want? You are adding the header size to the pointer, an address returned by calloc, attr_data

Copy link
Member Author

@Piloalucard Piloalucard Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.
Addresses are hexadecimal, integer values. Adding in this context by 1 means the address augments 1 byte as of:

  • Address A = 0x09
  • Address B = Null
  • Address B = Address A + 1 = 0x0A

So we write starting on the first position of our given memory address, it is an offset.

We add the memory header size to start writing in the buffer after the header size, which space is being occupied by the header.

It was refactored for readability.

Copy link
Contributor

@amissael95 amissael95 Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment to specify it? I think it does not hurt. You can use something like the explanation you gave:

// Adding the header size to start writing in the buffer after that size

Copy link
Member Author

@Piloalucard Piloalucard Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed


/* set attribute data */
if ( type == TC_MAM_APP_VENDER ) {
arch_strncpy((char *)attr_data + 5, t_attr->vender, attr_size+ TC_MAM_PAGE_HEADER_SIZE , attr_size);
} else if ( type == TC_MAM_APP_NAME ) {
arch_strncpy((char *)attr_data + 5, t_attr->app_name, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
} else if ( type == TC_MAM_APP_VERSION ) {
arch_strncpy((char *)attr_data + 5, t_attr->app_ver, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
} else if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
arch_strncpy((char *)attr_data + 5, t_attr->medium_label, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
} else if ( type == TC_MAM_TEXT_LOCALIZATION_IDENTIFIER ) {
attr_data[5] = t_attr->tli;
} else if ( type == TC_MAM_BARCODE ) {
arch_strncpy((char *)attr_data + 5, t_attr->barcode, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
} else if ( type == TC_MAM_APP_FORMAT_VERSION ) {
arch_strncpy((char *)attr_data + 5, t_attr->app_format_ver, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
} else if ( type == TC_MAM_LOCKED_MAM ) {
attr_data[5] = t_attr->vollock;
} else if ( type == TC_MAM_MEDIA_POOL) {
arch_strncpy((char *)attr_data + 5, t_attr->media_pool, attr_size + TC_MAM_PAGE_HEADER_SIZE, attr_size);
switch (type) {
case TC_MAM_APP_VENDER:
len = strnlen(t_attr->vender, attr_size);
memcpy(data, t_attr->vender, len);
break;

case TC_MAM_APP_NAME:
len = strnlen(t_attr->app_name, attr_size);
memcpy(data, t_attr->app_name, len);
break;

case TC_MAM_APP_VERSION:
len = strnlen(t_attr->app_ver, attr_size);
memcpy(data, t_attr->app_ver, len);
break;

case TC_MAM_USER_MEDIUM_LABEL:
len = strnlen(t_attr->medium_label, attr_size);
memcpy(data, t_attr->medium_label, len);
break;

case TC_MAM_TEXT_LOCALIZATION_IDENTIFIER:
data[0] = t_attr->tli;
break;

case TC_MAM_BARCODE:
len = strnlen(t_attr->barcode, attr_size);
memcpy(data, t_attr->barcode, len);
break;

case TC_MAM_APP_FORMAT_VERSION:
len = strnlen(t_attr->app_format_ver, attr_size);
memcpy(data, t_attr->app_format_ver, len);
break;

case TC_MAM_LOCKED_MAM:
data[0] = t_attr->vollock;
break;

case TC_MAM_MEDIA_POOL:
len = strnlen(t_attr->media_pool, attr_size);
memcpy(data, t_attr->media_pool, len);
break;
}

ret = dev->backend->write_attribute(dev->backend_data,
0, /* partition */
attr_data,
(attr_size + TC_MAM_PAGE_HEADER_SIZE));
0,
attr_data,
attr_size + TC_MAM_PAGE_HEADER_SIZE);

if (ret < 0)
ltfsmsg(LTFS_ERR, 17205E, type, "tape_set_attribute_to_cm");

free(attr_data);
return ret;

}

/**
Expand Down Expand Up @@ -3218,12 +3261,13 @@ int tape_get_attribute_from_cm(struct device_data *dev, struct tape_attr *t_attr
{
int ret;
int attr_len;
unsigned char* attr_data = NULL;

CHECK_ARG_NULL(dev, -LTFS_NULL_ARG);
CHECK_ARG_NULL(t_attr, -LTFS_NULL_ARG);

switch (type) {
case TC_MAM_APP_VENDER:
case TC_MAM_APP_VENDER:
attr_len = TC_MAM_APP_VENDER_SIZE;
break;
case TC_MAM_APP_NAME:
Expand Down Expand Up @@ -3256,14 +3300,14 @@ int tape_get_attribute_from_cm(struct device_data *dev, struct tape_attr *t_attr
break;
}

unsigned char *attr_data = NULL;
attr_data = malloc(sizeof(char*) * (attr_len + TC_MAM_PAGE_HEADER_SIZE));
if (attr_data == NULL) return -LTFS_NO_MEMORY;
int attr_size = sizeof(char) * (attr_len + TC_MAM_PAGE_HEADER_SIZE);
attr_data = (unsigned char*)malloc(attr_size);
if (!attr_data) return -LTFS_NO_MEMORY;
ret = dev->backend->read_attribute(dev->backend_data,
0, /* partition */
type,
attr_data,
sizeof(attr_data));
0, /* partition */
type,
attr_data,
attr_size);

if (ret == 0) {
uint16_t id = ltfs_betou16(attr_data);
Expand Down Expand Up @@ -3472,10 +3516,10 @@ int update_tape_attribute(struct ltfs_volume *vol, const char *new_value, int ty
if (ret < 0) {
if ( type == TC_MAM_USER_MEDIUM_LABEL ) {
memset(vol->t_attr->medium_label, '\0', TC_MAM_USER_MEDIUM_LABEL_SIZE + 1);
arch_strncpy_auto(vol->t_attr->medium_label, pre_attr, strlen(pre_attr));
arch_strcpy_auto(vol->t_attr->medium_label, pre_attr);
} else if (type == TC_MAM_BARCODE) {
memset(vol->t_attr->barcode, '\0', TC_MAM_BARCODE_SIZE + 1);
arch_strncpy_auto(vol->t_attr->barcode, pre_attr, strlen(pre_attr));
arch_strcpy_auto(vol->t_attr->barcode, pre_attr);
}
}

Expand Down
Loading
Loading