Isobus and j1939acd fixes and improvements (#635)

The first three patches are from PR
https://github.com/linux-can/can-utils/pull/629 that was stuck due to
the review process. The review questions of commit
https://github.com/linux-can/can-utils/pull/629#pullrequestreview-4361812324
have been addressed in this PR (see commit message)

The most important fix that has an impact on the data structures on the
CAN bus is the last one ("isobusfs: pack the protocol structures").

"The isobusfs protocol structures describe the message layout defined in
ISO 11783-13 but were not packed. The compiler added padding bytes in
front of 16/32 bit values which follow an odd number of bytes, e.g. the
Read File Request (8 bytes according to C.3.5.2) had a size of 10 bytes
and the count value at the wrong position. Server and client of the
can-utils used the same structures and understood each other but the
messages were not compatible to other ISOBUS devices."

So this has to be checked and reviewed carefully as it breaks the former
protocol of can-utils (but likely fits the rest of the world).

isobusfs/isobusfs_cli.c

@@ -671,6 +671,10 @@ int main(int argc, char *argv[])
 
 	while (1) {
 		ret = isobusfs_cli_process_events_and_tasks(priv);
+		if (ret == ISOBUSFS_CLI_RET_EXIT) {
+			ret = 0;
+			break;
+		}
 		if (ret)
 			break;
 	}

isobusfs/isobusfs_cli.h

@@ -13,6 +13,9 @@
 #define ISOBUSFS_CLI_MAX_EPOLL_EVENTS		10
 #define ISOBUSFS_CLI_DEFAULT_WAIT_TIMEOUT_MS	1000 /* ms */
 
+/* internal return codes, not errno values */
+#define ISOBUSFS_CLI_RET_EXIT			1
+
 enum isobusfs_cli_state {
 	ISOBUSFS_CLI_STATE_CONNECTING,
 	ISOBUSFS_CLI_STATE_IDLE,

isobusfs/isobusfs_cli_dh.c

@@ -43,7 +43,7 @@ int isobusfs_cli_ccd_req(struct isobusfs_priv *priv, const char *name,
 	req->tan = isobusfs_cli_get_next_tan(priv);
 
 	memcpy(&req->name[0], name, name_len);
-	req->name_len = name_len;
+	req->name_len = htole16(name_len);
 
 	if (padding_size) {
 		/* Fill the rest of the res structure with 0xff */

isobusfs/isobusfs_cli_fa.c

@@ -308,7 +308,7 @@ int isobusfs_cli_fa_of_req(struct isobusfs_priv *priv, const char *name,
 	req->tan = isobusfs_cli_get_next_tan(priv);
 	req->flags = flags;
 	memcpy(&req->name[0], name, name_len);
-	req->name_len = name_len;
+	req->name_len = htole16(name_len);
 
 	if (padding_size) {
 		/* Fill the rest of the res structure with 0xff */

isobusfs/isobusfs_cli_int.c

@@ -16,6 +16,12 @@
 #define MAX_COMMAND_LENGTH 256
 
 #define MAX_DISPLAY_FILENAME_LENGTH 100
+/*
+ * ISO 11783-13:2021 B.21 minimal directory entry payload size in bytes:
+ * 1 (name length) + 1 (min name byte) + 1 (attributes) +
+ * 2 (date) + 2 (time) + 4 (size).
+ */
+#define ISOBUSFS_MIN_DIR_ENTRY_SIZE (1 + 1 + 1 + 2 + 2 + 4)
 
 struct command_mapping {
 	const char *command;
@@ -66,8 +72,8 @@ static int cmd_help(struct isobusfs_priv *priv, const char *options)
 static int cmd_exit(struct isobusfs_priv *priv, const char *options)
 {
 	pr_int("exit interactive mode\n");
-	/* Return -EINTR to indicate the program should exit */
+
-	return -EINTR;
+	return ISOBUSFS_CLI_RET_EXIT;
 }
 
 static int cmd_dmesg(struct isobusfs_priv *priv, const char *options)
@@ -510,8 +516,12 @@ isobusfs_cli_ls_handle_open_dir_sent(struct isobusfs_priv *priv,
 
 	ctx->handle = res->handle;
 
+	ctx->offset = 0;
+	ctx->entry_count = 0;
+
 	ret = isobusfs_cli_send_and_register_fa_sf_event(priv, ctx->handle,
-							 0, ctx->entry_count,
+							 ISOBUSFS_FA_SEEK_SET,
+							 ctx->offset,
 							 cb, ctx);
 	if (ret)
 		pr_int("Failed to send seek file request: %i\n", ret);
@@ -530,7 +540,7 @@ isobusfs_cli_ls_handle_seek_dir_sent(struct isobusfs_priv *priv,
 {
 	isobusfs_event_callback cb = isobusfs_cli_ls_event_callback;
 	struct isobusfs_fa_seekf_res *res =
-		(struct isobusfs_fa_seekf_res *)msg;
+		(struct isobusfs_fa_seekf_res *)msg->buf;
 	uint16_t count;
 	int ret;
 
@@ -543,8 +553,10 @@ isobusfs_cli_ls_handle_seek_dir_sent(struct isobusfs_priv *priv,
 		goto error;
 	}
 
-	/* set max possible number fitting in to 16bits */
+	/* ISO 11783-13:2021 C.3.5.2: count is number of directory entries. */
-	count = UINT16_MAX;
+	count = ISOBUSFS_MAX_DATA_LENGH / ISOBUSFS_MIN_DIR_ENTRY_SIZE;
+	if (!count)
+		count = 1;
 	ctx->request_count = count;
 
 	ret = isobusfs_cli_send_and_register_fa_rf_event(priv, ctx->handle,
@@ -616,8 +628,8 @@ static bool isobusfs_cli_extract_directory_entry(const uint8_t *buffer,
 						 uint16_t *file_time,
 						 uint32_t *file_size)
 {
+	size_t entry_total_len, copy_len;
 	uint8_t filename_length;
-	size_t entry_total_len;
 
 	if (*pos + 2 > buffer_length) {
 		pr_int("Error: Incomplete data in buffer\n");
@@ -633,8 +645,14 @@ static bool isobusfs_cli_extract_directory_entry(const uint8_t *buffer,
 	}
 
 	(*pos)++;
-	strncpy(filename, (const char *)buffer + *pos, filename_length);
+
-	filename[filename_length] = '\0';
+	if (filename_length > ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH)
+		copy_len = ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH;
+	else
+		copy_len = filename_length;
+
+	strncpy(filename, (const char *)buffer + *pos, copy_len);
+	filename[copy_len] = '\0';
 	*pos += filename_length;
 	if (filename_length > MAX_DISPLAY_FILENAME_LENGTH) {
 		/* Truncate the filename and replace the last character
@@ -688,15 +706,17 @@ isobusfs_cli_print_directory_entry(struct isobusfs_cli_ls_context *ctx,
 static void
 isobusfs_cli_print_directory_entries(struct isobusfs_cli_ls_context *ctx,
 				     const uint8_t *buffer,
-				     size_t buffer_length)
+				     size_t buffer_length,
+				     uint16_t max_entries)
 {
 	char filename[ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH + 1];
 	uint16_t file_date, file_time;
 	uint32_t file_size;
 	uint8_t attributes;
 	size_t pos = 0;
+	uint16_t entries = 0;
 
-	while (pos < buffer_length) {
+	while (pos < buffer_length && entries < max_entries) {
 		if (!isobusfs_cli_extract_directory_entry(buffer, buffer_length,
 							  &pos, filename,
 							  &attributes,
@@ -709,6 +729,7 @@ isobusfs_cli_print_directory_entries(struct isobusfs_cli_ls_context *ctx,
 						   file_date, file_time,
 						   file_size);
 		ctx->entry_count++;
+		entries++;
 	}
 }
 
@@ -721,26 +742,42 @@ isobusfs_cli_ls_handle_read_dir_sent(struct isobusfs_priv *priv,
 		(struct isobusfs_read_file_response *)msg->buf;
 	size_t buffer_length = msg->len - sizeof(*res);
 	isobusfs_event_callback cb;
+	size_t entries_before;
+	size_t entries_in_message;
 	uint16_t count;
 	int ret;
 
 	pr_debug("< rx: Read File Response. Error code: %i", res->error_code);
+
 	if (isobusfs_cli_int_is_error(priv, 0, res->error_code, res->tan))
 		goto error;
 
 	count = le16toh(res->count);
-	if (count && count != buffer_length) {
+	if (count && buffer_length) {
-		pr_int("Buffer length mismatch: %u != %zu\n", count,
+		entries_before = ctx->entry_count;
-		       buffer_length);
+		isobusfs_cli_print_directory_entries(ctx, res->data,
-		goto error;
+						     buffer_length, count);
+		entries_in_message = ctx->entry_count - entries_before;
+	} else {
+		entries_in_message = 0;
 	}
 
-	if (count)
+	if (count != entries_in_message) {
-		isobusfs_cli_print_directory_entries(ctx, res->data,
+		pr_warn("Directory entry count mismatch: server sent %u, parsed %zu\n",
-						     buffer_length);
+			count, entries_in_message);
+		/* Continue processing if we parsed at least some entries.
+		 * Strict validation would reject valid responses if parsing
+		 * fails partway through due to corruption.
+		 */
+		if (entries_in_message == 0 && count > 0) {
+			pr_int("Error: Failed to parse any directory entries\n");
+			goto error;
+		}
+	}
 
 	cb = isobusfs_cli_ls_event_callback;
-	if (count) {
+
+	if (res->error_code == ISOBUSFS_ERR_END_OF_FILE) {
 		ret = isobusfs_cli_send_and_register_fa_cf_event(priv,
 								 ctx->handle,
 								 cb, ctx);
@@ -750,8 +787,20 @@ isobusfs_cli_ls_handle_read_dir_sent(struct isobusfs_priv *priv,
 		}
 
 		ctx->state = ISOBUSFS_CLI_LS_STATE_CLOSE_DIR_SENT;
-	} else {
+		return;
+	}
+
+	if (!count) {
+		pr_int("Error: zero-length read without EOF\n");
+		goto error;
+	}
+
+	/*
+	 * Directory seek offset is entry index, not byte offset.
+	 * Server side seek rewinds and skips "offset" entries.
+	 */
 	ctx->offset = ctx->entry_count;
+
 	ret = isobusfs_cli_send_and_register_fa_sf_event(priv,
 							 ctx->handle, 0,
 							 ctx->offset,
@@ -760,11 +809,9 @@ isobusfs_cli_ls_handle_read_dir_sent(struct isobusfs_priv *priv,
 		pr_int("Failed to send seek file request: %i\n", ret);
 
 	ctx->state = ISOBUSFS_CLI_LS_STATE_SEEK_DIR_SENT;
-	}
 
 	return;
 error:
-
 	ctx->state = ISOBUSFS_CLI_LS_STATE_ERROR;
 }
 

isobusfs/isobusfs_cli_selftests.c

@@ -603,23 +603,24 @@ struct isobusfs_cli_test_rf_path {
 	uint8_t flags;
 	uint32_t offset;
 	uint32_t read_size;
+	uint32_t expected_size;
 	bool expect_pass;
 };
 
 static struct isobusfs_cli_test_rf_path test_rf_patterns[] = {
 	/* expected result \\vol1\dir1\dir2\ */
-	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 0, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 0, 0, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 0, 1, 1, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 1, 1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 1, 1, 1, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 2, 1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 2, 1, 1, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 3, 1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1k", 0, 3, 1, 1, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8, 8, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8 * 100, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, 8 * 100, 8 * 100, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 100, 8 * 100, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 100, 8 * 100, 8 * 100, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH, ISOBUSFS_MAX_DATA_LENGH, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, (ISOBUSFS_MAX_DATA_LENGH & ~3) + 16, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH + 1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, ISOBUSFS_MAX_DATA_LENGH + 1, ISOBUSFS_MAX_DATA_LENGH + 1, true },
-	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, -1, true },
+	{ "\\\\vol1\\dir1\\dir2\\file1m", 0, 0, UINT32_MAX, 1024 * 1024, true },
 };
 
 size_t current_rf_pattern_test;
@@ -678,7 +679,8 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
 		&test_rf_patterns[current_rf_pattern_test];
 	uint32_t actual_sum, expected_sum;
 	struct timespec current_time;
-	ssize_t remaining_size, read_size;
+	int64_t remaining_size;
+	ssize_t read_size;
 	int ret;
 
 	clock_gettime(CLOCK_MONOTONIC, &current_time);
@@ -801,16 +803,23 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
 		free(priv->read_data);
 		priv->read_data = NULL;
 
-		remaining_size = (tp->offset + tp->read_size) -
+		remaining_size = ((int64_t)tp->offset + tp->read_size) -
-				(priv->read_offset + priv->read_data_len);
+				((int64_t)priv->read_offset + priv->read_data_len);
-		pr_debug("remaining_size: %zd, read_offset: %zu, read_data_len: %zu, test read size: %zu, test offset %zu",
+		pr_debug("remaining_size: %lld, read_offset: %u, read_data_len: %zu, test read size: %u, test offset %u",
-			 remaining_size, priv->read_offset, priv->read_data_len,
+			 (long long)remaining_size, priv->read_offset, priv->read_data_len,
 			 tp->read_size, tp->offset);
 		if (remaining_size < 0) {
 			pr_err("pattern test failed: %s. Read size is too big",
 			       tp->path_name);
 			ret = -EINVAL;
 			goto test_fail;
+		} else if (remaining_size == 0) {
+			if (tp->read_size > tp->expected_size) {
+				pr_err("read test failed: %s. Server returned more data than expected file size (%u > %u)",
+				       tp->path_name, tp->read_size, tp->expected_size);
+				ret = -EINVAL;
+				goto test_fail;
+			}
 		} else if (remaining_size > 0 && priv->read_data_len != 0) {
 			priv->read_offset += priv->read_data_len;
 
@@ -825,12 +834,21 @@ static int isobusfs_cli_test_rf_req(struct isobusfs_priv *priv, bool *complete)
 				goto test_fail;
 			test_start_time = current_time;
 			break;
-		} else if (remaining_size > 0 && priv->read_data_len == 0 && tp->expect_pass) {
+		} else if (remaining_size > 0 && priv->read_data_len == 0) {
-			pr_err("read test failed: %s. Read size is zero, but expected more data: %zd",
+			if (tp->read_size > tp->expected_size &&
-			       tp->path_name, remaining_size);
+			    tp->read_size - remaining_size == tp->expected_size) {
+				/* this is acceptable case when read size
+				 * is larger than actual file size
+				 */
+				pr_info("read test passed: %s. Reached end of file as expected.",
+					tp->path_name);
+			} else {
+				pr_err("read test failed: %s. Server returned zero bytes, but expected more data: %lld",
+				       tp->path_name, (long long)remaining_size);
 				ret = -EINVAL;
 				goto test_fail;
 			}
+		}
 
 		/* fall troth */
 	case ISOBUSFS_CLI_STATE_TEST_CLEANUP:

isobusfs/isobusfs_cmn_cm.h

@@ -39,7 +39,7 @@ struct isobusfs_cm_fss {
 	uint8_t status;
 	uint8_t num_open_files;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 /**
  * C.1.3 Client Connection Maintenance
@@ -58,7 +58,7 @@ struct isobusfs_cm_ccm {
 	uint8_t fs_function;
 	uint8_t version;
 	uint8_t reserved[6];
-};
+} __attribute__((__packed__));
 
 /**
  * C.1.4 Get File Server Properties
@@ -75,7 +75,7 @@ struct isobusfs_cm_ccm {
 struct isobusfs_cm_get_fs_props_req {
 	uint8_t fs_function;
 	uint8_t reserved[7];
-};
+} __attribute__((__packed__));
 
 /* File Server Capabilities */
 /* server support removable volumes */
@@ -104,7 +104,7 @@ struct isobusfs_cm_get_fs_props_resp {
 	uint8_t max_open_files;
 	uint8_t fs_capabilities;
 	uint8_t reserved[4];
-};
+} __attribute__((__packed__));
 
 #define ISOBUSFS_VOL_MODE_PREP_TO_REMOVE	BIT(1)
 #define ISOBUSFS_VOL_MODE_USED_BY_CLIENT	BIT(0)
@@ -130,7 +130,7 @@ struct isobusfs_cm_vol_stat_req {
 	uint8_t volume_mode;
 	__le16 name_len;
 	char name[ISOBUSFS_MAX_VOLUME_NAME_LENGTH];
-};
+} __attribute__((__packed__));
 
 
 enum isobusfs_vol_status {
@@ -169,7 +169,7 @@ struct isobusfs_cm_vol_stat_res {
 	uint8_t error_code;
 	__le16 name_len;
 	char name[ISOBUSFS_MAX_VOLUME_NAME_LENGTH];
-};
+} __attribute__((__packed__));
 
 
 #endif /* ISOBUSFS_CMN_CM_H */

isobusfs/isobusfs_cmn_dh.h

@@ -23,7 +23,7 @@ struct isobusfs_dh_get_cd_req {
 	uint8_t fs_function;
 	uint8_t tan;
 	uint8_t reserved[6];
-};
+} __attribute__((__packed__));
 
 /**
  * C.2.2.3 Get Current Directory Response
@@ -50,7 +50,7 @@ struct isobusfs_dh_get_cd_res {
 	__le32 free_space;
 	__le16 name_len;
 	uint8_t name[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.2.3.2 Change Current Directory Request
@@ -71,7 +71,7 @@ struct isobusfs_dh_ccd_req {
 	uint8_t tan;
 	__le16 name_len;
 	uint8_t name[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.2.3.3 Change Current Directory Response
@@ -101,7 +101,7 @@ struct isobusfs_dh_ccd_res {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 
 #endif /* _ISOBUSFS_CMN_DH_H */

isobusfs/isobusfs_cmn_fa.h

@@ -105,7 +105,7 @@ struct isobusfs_fa_openf_req {
 	uint8_t flags;
 	__le16 name_len;
 	uint8_t name[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.3.3 Open File Response
@@ -141,7 +141,7 @@ struct isobusfs_fa_openf_res {
 	uint8_t handle;
 	uint8_t attributes;
 	uint8_t reserved[3];
-};
+} __attribute__((__packed__));
 
 /* B.17 Position mode */
 /* From the beginning of the file */
@@ -168,7 +168,7 @@ struct isobusfs_fa_seekf_req {
 	uint8_t handle;
 	uint8_t position_mode;
 	__le32 offset;
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.4.2 Seek File Response
@@ -195,7 +195,7 @@ struct isobusfs_fa_seekf_res {
 	uint8_t error_code;
 	uint8_t reserved;
 	__le32 position;
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.5.2 Read File Request
@@ -220,7 +220,7 @@ struct isobusfs_fa_readf_req {
 	uint8_t handle;
 	__le16 count;
 	uint8_t reserved[3];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.5.3 Read File Response (Handle-referenced file)
@@ -250,7 +250,7 @@ struct isobusfs_read_file_response {
 	uint8_t error_code;
 	__le16 count;
 	uint8_t data[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.5.4 Read Directory Response (Handle-referenced directory)
@@ -280,7 +280,7 @@ struct isobusfs_read_dir_response {
 	uint8_t error_code;
 	__le16 count;
 	uint8_t data[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.6.2 Write File Request
@@ -303,7 +303,7 @@ struct isobusfs_write_file_request {
 	uint8_t handle;
 	__le16 count;
 	uint8_t data[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.6.3 Write File Response
@@ -333,7 +333,7 @@ struct isobusfs_write_file_response {
 	uint8_t error_code;
 	__le16 count;
 	uint8_t reserved[3];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.7.1 Close File Request
@@ -354,7 +354,7 @@ struct isobusfs_close_file_request {
 	uint8_t tan;
 	uint8_t handle;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 /**
  * C.3.7.2 Close File Response
@@ -378,7 +378,7 @@ struct isobusfs_close_file_res {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[6];
-};
+} __attribute__((__packed__));
 
 
 #endif /* _LINUX_ISOBUS_FS_H */

isobusfs/isobusfs_cmn_fh.h

@@ -32,7 +32,7 @@ struct isobusfs_move_file_request {
 	/* Variable length data follows */
 	/* uint8_t src_path[]; */
 	/* uint8_t dst_path[]; */
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.2.3 Move File Response
@@ -64,7 +64,7 @@ struct isobusfs_move_file_response {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.3.1 Delete File Request
@@ -87,7 +87,7 @@ struct isobusfs_delete_file_request {
 	uint8_t mode;
 	__le16 path_len;
 	uint8_t path[];
-};
+} __attribute__((__packed__));
 
 
 /**
@@ -118,7 +118,7 @@ struct isobusfs_delete_file_response {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.4.2 Get File Attributes Request
@@ -139,7 +139,7 @@ struct isobusfs_get_file_attributes_request {
 	uint8_t tan;
 	__le16 pathname_len;
 	uint8_t pathname[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.4.3 Get File Attributes Response
@@ -171,7 +171,7 @@ struct isobusfs_get_file_attributes_response {
 	uint8_t error_code;
 	uint8_t attributes;
 	__le32 file_size;
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.5.2 Set File Attributes Request
@@ -194,7 +194,7 @@ struct isobusfs_set_file_attributes_request {
 	uint8_t attributes;
 	__le16 name_length;
 	uint8_t name[]; /* Variable length */
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.5.3 Set File Attributes Response
@@ -225,7 +225,7 @@ struct isobusfs_set_file_attributes_response {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[5];
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.6.2 Get File Date & Time Request
@@ -246,7 +246,7 @@ struct isobusfs_get_file_date_time_request {
 	uint8_t tan;
 	__le16 name_length;
 	uint8_t name[]; /* Variable length */
-};
+} __attribute__((__packed__));
 
 /**
  * C.4.6.2 Get File Date & Time Response
@@ -278,6 +278,6 @@ struct isobusfs_get_file_date_time_response {
 	uint8_t error_code;
 	__le16 date;
 	__le16 time;
-};
+} __attribute__((__packed__));
 
 #endif /* _LINUX_ISOBUS_FS_H */

isobusfs/isobusfs_cmn_va.h

@@ -29,7 +29,7 @@ struct isobusfs_va_init_vol_req {
 	uint8_t volume_flags;
 	__le16 name_len;
 	uint8_t name[];
-};
+} __attribute__((__packed__));
 
 /**
  * C.5.2.3 Initialize Volume Response
@@ -62,6 +62,6 @@ struct isobusfs_va_init_vol_res {
 	uint8_t tan;
 	uint8_t error_code;
 	uint8_t reserved[4];
-};
+} __attribute__((__packed__));
 
 #endif /* _ISOBUSFS_CMN_VA_H */

isobusfs/isobusfs_srv_cm.c

@@ -20,6 +20,7 @@
  */
 
 #include <errno.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
@@ -473,8 +474,14 @@ static int isobusfs_srv_process_volume_status_request(struct isobusfs_srv_priv *
 
 	req_name_len = le16toh(req->name_len);
 
-	pr_debug("< rx volume status request. mode: %x, length: %d, name: %s",
+	/* the volume name has to fit into the received message */
-		 req->volume_mode, req_name_len, req->name);
+	if (req_name_len > msg->len - offsetof(struct isobusfs_cm_vol_stat_req, name)) {
+		pr_warn("volume name length exceeds message length");
+		return ISOBUSFS_ERR_MALFORMED_REQUEST;
+	}
+
+	pr_debug("< rx volume status request. mode: %x, length: %zu, name: %.*s",
+		 req->volume_mode, req_name_len, (int)req_name_len, req->name);
 
 	client = isobusfs_srv_get_client_by_msg(priv, msg);
 	if (!client) {

isobusfs/isobusfs_srv_dh.c

@@ -170,10 +170,13 @@ static int isobusfs_srv_dh_current_dir_res(struct isobusfs_srv_priv *priv,
 		switch (ret) {
 		case -ENOENT:
 			error_code = ISOBUSFS_ERR_FILE_ORPATH_NOT_FOUND;
+			break;
 		case -ENOMEDIUM:
 			error_code = ISOBUSFS_ERR_VOLUME_NOT_INITIALIZED;
+			break;
 		case -ENOMEM:
 			error_code = ISOBUSFS_ERR_OUT_OF_MEM;
+			break;
 		default:
 			error_code = ISOBUSFS_ERR_OTHER;
 		}
@@ -657,15 +660,22 @@ static int isobusfs_srv_dh_ccd_res(struct isobusfs_srv_priv *priv,
 	uint8_t error_code = ISOBUSFS_ERR_SUCCESS;
 	struct isobusfs_srv_client *client;
 	struct isobusfs_dh_ccd_res res;
+	uint16_t name_len = le16toh(req->name_len);
 	size_t abs_path_len;
 	char *abs_path;
 	int ret;
 
+	/* the path name has to fit into the received message */
+	if (name_len > msg->len - sizeof(*req)) {
+		pr_warn("path name length exceeds message length");
+		return -EINVAL;
+	}
+
 	/*
 	 * We assume, the relative path stored in res->name is not longer
 	 * than absolute path
 	 */
-	if (req->name_len > ISOBUSFS_SRV_MAX_PATH_LEN) {
+	if (name_len > ISOBUSFS_SRV_MAX_PATH_LEN) {
 		pr_warn("path too long");
 		return -EINVAL;
 	}
@@ -684,10 +694,10 @@ static int isobusfs_srv_dh_ccd_res(struct isobusfs_srv_priv *priv,
 	}
 
 	pr_debug("< rx change current directory request from client 0x%2x: %.*s. Current directory: %s",
-		 client->addr, req->name_len, req->name, client->current_dir);
+		 client->addr, name_len, req->name, client->current_dir);
 	/* Normalize provided string and convert it to absolute ISOBUS FS path */
 	ret = isobusfs_convert_relative_to_absolute(priv, client->current_dir,
-						    (char *)req->name, req->name_len,
+						    (char *)req->name, name_len,
 						    abs_path, abs_path_len);
 	if (ret < 0)
 		goto process_error;

isobusfs/isobusfs_srv_fa.c

@@ -45,6 +45,9 @@ static int isobusfs_srv_add_file(struct isobusfs_srv_priv *priv,
 	}
 
 	priv->handles[j].path = strdup(path);
+	if (!priv->handles[j].path)
+		return -ENOMEM;
+
 	priv->handles[j].fd = fd;
 	priv->handles[j].dir = dir;
 
@@ -108,6 +111,28 @@ isobusfs_srv_get_handle(struct isobusfs_srv_priv *priv, int handle)
 	return &priv->handles[handle];
 }
 
+/*
+ * Return the handle only when it is in use and has been opened by the
+ * requesting client. Otherwise return NULL.
+ */
+static struct isobusfs_srv_handles *
+isobusfs_srv_get_client_handle(struct isobusfs_srv_priv *priv,
+			       struct isobusfs_srv_client *client, int handle)
+{
+	struct isobusfs_srv_handles *hdl = isobusfs_srv_get_handle(priv, handle);
+	unsigned int i;
+
+	if (!hdl || !hdl->path)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(hdl->clients); i++) {
+		if (hdl->clients[i] == client)
+			return hdl;
+	}
+
+	return NULL;
+}
+
 static int isobusfs_srv_release_handle(struct isobusfs_srv_priv *priv,
 				       struct isobusfs_srv_client *client,
 				       int handle)
@@ -137,6 +162,7 @@ static int isobusfs_srv_release_handle(struct isobusfs_srv_priv *priv,
 					closedir(hdl->dir);
 				else
 					close(hdl->fd);
+				free(hdl->path);
 				memset(hdl, 0, sizeof(*hdl));
 				priv->handles_count--;
 			}
@@ -495,15 +521,119 @@ static int check_access_with_base(const char *base_dir,
 	return access(full_path, mode);
 }
 
+/*
+ * ISO 11783-13:2021 B.21 and B.15:
+ * Filters directory entries that can be returned in a Read File response
+ * for directory handles while collecting the attributes and name length.
+ */
+/**
+ * isobusfs_srv_dir_entry_visible() - Filter visible directory entries.
+ * @handle: Directory handle for access checks.
+ * @entry: Directory entry to inspect.
+ * @file_stat: Stat buffer to fill for the entry.
+ * @entry_name_len: Returns entry name length on success.
+ * @attributes: Returns computed attributes on success.
+ *
+ * ISO 11783-13:2021 B.21 and B.15 define the directory entry layout and
+ * attributes. This helper skips entries that are not readable or too long
+ * and returns attributes for the entry that will be serialized.
+ *
+ * Return: true when the entry should be emitted, false otherwise.
+ */
+static bool isobusfs_srv_dir_entry_visible(struct isobusfs_srv_handles *handle,
+					   const struct dirent *entry,
+					   struct stat *file_stat,
+					   size_t *entry_name_len,
+					   uint8_t *attributes)
+{
+	if (check_access_with_base(handle->path, entry->d_name, R_OK) != 0)
+		return false;
+
+	if (fstatat(handle->fd, entry->d_name, file_stat, 0) < 0)
+		return false;
+
+	*entry_name_len = strlen(entry->d_name);
+	if (*entry_name_len > ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH)
+		return false;
+
+	if (S_ISDIR(file_stat->st_mode))
+		*attributes |= ISOBUSFS_ATTR_DIRECTORY;
+	if (check_access_with_base(handle->path, entry->d_name, W_OK) != 0)
+		*attributes |= ISOBUSFS_ATTR_READ_ONLY;
+
+	return true;
+}
+
+/*
+ * ISO 11783-13:2021 C.3.4.2 and C.3.5.2:
+ * Directory offsets/counts are in entries, so advance the directory stream
+ * by visible entries only and report EOF if the entry offset is past the end.
+ */
+/**
+ * isobusfs_srv_dir_skip_entries() - Advance directory stream by entries.
+ * @handle: Directory handle to reposition.
+ * @offset: Entry offset to seek to.
+ *
+ * ISO 11783-13:2021 C.3.4.2 and C.3.5.2 state directory offsets/counts are in
+ * entries. This helper advances by visible entries only.
+ *
+ * Return: ISOBUSFS_ERR_SUCCESS or a protocol error code.
+ */
+static int isobusfs_srv_dir_skip_entries(struct isobusfs_srv_handles *handle,
+					 int32_t offset)
+{
+	struct dirent *entry;
+	int32_t skipped = 0;
+
+	rewinddir(handle->dir);
+
+	while (skipped < offset && (entry = readdir(handle->dir)) != NULL) {
+		struct stat file_stat;
+		size_t entry_name_len = 0;
+		uint8_t attributes = 0;
+
+		if (!isobusfs_srv_dir_entry_visible(handle, entry, &file_stat,
+						    &entry_name_len,
+						    &attributes))
+			continue;
+
+		skipped++;
+	}
+
+	if (offset > 0 && skipped < offset)
+		return ISOBUSFS_ERR_END_OF_FILE;
+
+	return ISOBUSFS_ERR_SUCCESS;
+}
+
+/**
+ * isobusfs_srv_read_directory() - Read directory entries into a response buffer.
+ * @handle: Directory handle to read.
+ * @buffer: Output buffer for directory entries.
+ * @max_bytes: Maximum payload bytes allowed in the response.
+ * @max_entries: Maximum number of entries to return.
+ * @readed_size: Returns payload size in bytes.
+ * @entries_read: Returns number of entries serialized.
+ *
+ * ISO 11783-13:2021 C.3.5.2: directory Count is entry based. This function
+ * encodes up to @max_entries entries while respecting @max_bytes.
+ *
+ * Return: ISOBUSFS_ERR_SUCCESS or a protocol error code.
+ */
 static int isobusfs_srv_read_directory(struct isobusfs_srv_handles *handle,
-				       uint8_t *buffer, size_t count,
+				       uint8_t *buffer, size_t max_bytes,
-				       ssize_t *readed_size)
+				       uint16_t max_entries,
+				       ssize_t *readed_size,
+				       uint16_t *entries_read)
 {
 	DIR *dir = handle->dir;
 	struct dirent *entry;
 	size_t pos = 0;
+	int ret;
 
 	/*
+	 * ISO 11783-13:2021 C.3.5.2:
+	 * Directory offsets are entry indices, not byte positions.
 	 * Position the directory stream to the previously stored offset (handle->dir_pos).
 	 *
 	 * Handling Changes in Directory Contents:
@@ -519,13 +649,18 @@ static int isobusfs_srv_read_directory(struct isobusfs_srv_handles *handle,
 	 *   either returning an error or restarting from the beginning of the directory, depending
 	 *   on the application's requirements.
 	 */
-	for (int i = 0; i < handle->dir_pos &&
+	if (handle->dir_pos < 0) {
-	     (entry = readdir(dir)) != NULL; i++) {
+		pr_err("Directory handle in invalid state (pos=%d)\n",
-		/* Iterating to the desired position */
+		       handle->dir_pos);
+		return ISOBUSFS_ERR_INVALID_HANDLE;
 	}
 
+	ret = isobusfs_srv_dir_skip_entries(handle, handle->dir_pos);
+	if (ret != ISOBUSFS_ERR_SUCCESS)
+		return ret;
+
 	/*
-	 * Directory Entry Layout:
+	 * Directory Entry Layout (ISO 11783-13:2021 B.21):
 	 * This loop reads directory entries and encodes them into a buffer.
 	 * Each entry in the buffer follows the format specified in ISO 11783-13:2021.
 	 *
@@ -550,26 +685,23 @@ static int isobusfs_srv_read_directory(struct isobusfs_srv_handles *handle,
 	 * The handle->dir_pos is incremented after processing each entry, marking
 	 * the current position in the directory stream for subsequent reads.
 	 */
-	while ((entry = readdir(dir)) != NULL) {
+	*entries_read = 0;
+	while ((entry = readdir(dir)) != NULL &&
+	       (*entries_read) < max_entries) {
 		size_t entry_name_len, entry_total_len;
 		__le16 file_date, file_time;
-		uint8_t attributes = 0;
 		struct stat file_stat;
+		uint8_t attributes = 0;
 		__le32 size;
 
-		if (check_access_with_base(handle->path, entry->d_name, R_OK) != 0)
+		if (!isobusfs_srv_dir_entry_visible(handle, entry, &file_stat,
-			continue; /* Skip this entry if it's not readable */
+						    &entry_name_len,
-
+						    &attributes))
-		if (fstatat(handle->fd, entry->d_name, &file_stat, 0) < 0)
-			continue; /* Skip this entry on error */
-
-		entry_name_len = strlen(entry->d_name);
-		if (entry_name_len > ISOBUSFS_MAX_DIR_ENTRY_NAME_LENGTH)
 			continue;
 
 		entry_total_len = 1 + entry_name_len + 1 + 2 + 2 + 4;
 
-		if (pos + entry_total_len > count)
+		if (pos + entry_total_len > max_bytes)
 			break;
 
 		buffer[pos++] = (uint8_t)entry_name_len;
@@ -577,10 +709,6 @@ static int isobusfs_srv_read_directory(struct isobusfs_srv_handles *handle,
 		memcpy(buffer + pos, entry->d_name, entry_name_len);
 		pos += entry_name_len;
 
-		if (S_ISDIR(file_stat.st_mode))
-			attributes |= ISOBUSFS_ATTR_DIRECTORY;
-		if (check_access_with_base(handle->path, entry->d_name, W_OK) != 0)
-			attributes |= ISOBUSFS_ATTR_READ_ONLY;
 		buffer[pos++] = attributes;
 
 		file_date = htole16(convert_to_file_date(file_stat.st_mtime));
@@ -594,9 +722,11 @@ static int isobusfs_srv_read_directory(struct isobusfs_srv_handles *handle,
 		size = htole32(file_stat.st_size);
 		memcpy(buffer + pos, &size, sizeof(size));
 		pos += sizeof(size);
+		(*entries_read)++;
 	}
 
 	*readed_size = pos;
+	handle->dir_pos += *entries_read;
 	return 0;
 }
 
@@ -609,13 +739,19 @@ static int isobusfs_srv_fa_rf_req(struct isobusfs_srv_priv *priv,
 	struct isobusfs_srv_client *client;
 	struct isobusfs_fa_readf_req *req;
 	ssize_t readed_size = 0;
+	uint16_t entries_read = 0;
 	uint8_t error_code = 0;
 	ssize_t send_size;
+	bool res_allocated = false;
+	size_t alloc_size;
+	size_t max_bytes = 0;
+	uint16_t count = 0;
 	int ret = 0;
-	int count;
+	bool is_dir = false;
 
 	req = (struct isobusfs_fa_readf_req *)msg->buf;
 	count = le16toh(req->count);
+	res = (struct isobusfs_read_file_response *)&res_fail[0];
 
 	pr_debug("< rx: Read File Request. tan: %d, handle: %d, count: %d",
 		 req->tan, req->handle, count);
@@ -627,17 +763,6 @@ static int isobusfs_srv_fa_rf_req(struct isobusfs_srv_priv *priv,
 	 * TODO: currently we are not able to detect support transport mode,
 	 * so ETP is assumed.
 	 */
-	if (count > ISOBUSFS_MAX_DATA_LENGH)
-		count = ISOBUSFS_MAX_DATA_LENGH;
-
-	res = malloc(sizeof(*res) + count);
-	if (!res) {
-		pr_warn("failed to allocate memory");
-		res = (struct isobusfs_read_file_response *)&res_fail[0];
-		error_code = ISOBUSFS_ERR_OUT_OF_MEM;
-		goto send_response;
-	}
-
 	client = isobusfs_srv_get_client_by_msg(priv, msg);
 	if (!client) {
 		pr_warn("client not found");
@@ -645,16 +770,42 @@ static int isobusfs_srv_fa_rf_req(struct isobusfs_srv_priv *priv,
 		goto send_response;
 	}
 
-	handle = isobusfs_srv_get_handle(priv, req->handle);
+	handle = isobusfs_srv_get_client_handle(priv, client, req->handle);
 	if (!handle) {
 		pr_warn("failed to find file with handle: %x", req->handle);
 		error_code = ISOBUSFS_ERR_FILE_ORPATH_NOT_FOUND;
+		goto send_response;
 	}
 
 	/* Determine whether to read a file or a directory */
 	if (handle->dir) {
-		ret = isobusfs_srv_read_directory(handle, res->data, count,
+		/* ISO 11783-13:2021 C.3.5.2: count is entry count for directories. */
-						  &readed_size);
+		is_dir = true;
+		max_bytes = ISOBUSFS_MAX_DATA_LENGH;
+	} else {
+		if (count > ISOBUSFS_MAX_DATA_LENGH)
+			count = ISOBUSFS_MAX_DATA_LENGH;
+		max_bytes = count;
+	}
+
+	/* the response is padded to ISOBUSFS_MIN_TRANSFER_LENGH (see below) */
+	alloc_size = sizeof(*res) + max_bytes;
+	if (alloc_size < ISOBUSFS_MIN_TRANSFER_LENGH)
+		alloc_size = ISOBUSFS_MIN_TRANSFER_LENGH;
+
+	res = malloc(alloc_size);
+	if (!res) {
+		pr_warn("failed to allocate memory");
+		res = (struct isobusfs_read_file_response *)&res_fail[0];
+		error_code = ISOBUSFS_ERR_OUT_OF_MEM;
+		goto send_response;
+	}
+	res_allocated = true;
+
+	if (is_dir) {
+		ret = isobusfs_srv_read_directory(handle, res->data, max_bytes,
+						  count, &readed_size,
+						  &entries_read);
 	} else {
 		ret = isobusfs_srv_read_file(handle, res->data, count,
 					     &readed_size);
@@ -663,7 +814,10 @@ static int isobusfs_srv_fa_rf_req(struct isobusfs_srv_priv *priv,
 	if (ret < 0) {
 		error_code = ret;
 		readed_size = 0;
-	} else if (count != 0 && readed_size == 0) {
+		entries_read = 0;
+	} else if (count != 0 &&
+		   ((is_dir && entries_read == 0) ||
+		    (!is_dir && readed_size == 0))) {
 		error_code = ISOBUSFS_ERR_END_OF_FILE;
 	}
 
@@ -673,11 +827,18 @@ send_response:
 					    ISOBUSFS_FA_F_READ_FILE_RES);
 	res->tan = req->tan;
 	res->error_code = error_code;
+	if (is_dir)
+		res->count = htole16(entries_read);
+	else
 		res->count = htole16(readed_size);
 
 	send_size = sizeof(*res) + readed_size;
-	if (send_size < ISOBUSFS_MIN_TRANSFER_LENGH)
+	if (send_size < ISOBUSFS_MIN_TRANSFER_LENGH) {
+		/* fill the unused bytes with 0xff */
+		memset((uint8_t *)res + send_size, 0xff,
+		       ISOBUSFS_MIN_TRANSFER_LENGH - send_size);
 		send_size = ISOBUSFS_MIN_TRANSFER_LENGH;
+	}
 
 	/* send to socket */
 	ret = isobusfs_srv_sendto(priv, msg, res, send_size);
@@ -690,6 +851,7 @@ send_response:
 		 error_code, isobusfs_error_to_str(error_code), readed_size);
 
 free_res:
+	if (res_allocated)
 		free(res);
 	return ret;
 }
@@ -754,19 +916,56 @@ static int isobusfs_srv_seek(struct isobusfs_srv_priv *priv,
 	return ISOBUSFS_ERR_SUCCESS;
 }
 
+/**
+ * isobusfs_srv_seek_directory() - Seek a directory by entry index.
+ * @handle: Directory handle to seek.
+ * @offset: Entry index to seek to.
+ *
+ * ISO 11783-13:2021 C.3.4.2 defines directory offsets as entry indices.
+ *
+ * Return: ISOBUSFS_ERR_SUCCESS or a protocol error code.
+ */
 static int isobusfs_srv_seek_directory(struct isobusfs_srv_handles *handle,
 				       int32_t offset)
 {
-	DIR *dir = fdopendir(handle->fd);
+	int32_t current_pos = handle->dir_pos;
+	int ret;
 
-	if (!dir)
+	if (!handle->dir)
 		return ISOBUSFS_ERR_OTHER;
 
-	rewinddir(dir);
+	/* Check if handle is in valid state */
+	if (current_pos < 0) {
+		pr_err("Cannot seek directory in invalid state (pos=%d)\n",
+		       current_pos);
+		return ISOBUSFS_ERR_INVALID_HANDLE;
+	}
 
-	for (int32_t i = 0; i < offset; i++) {
+	/* Validate requested offset */
-		if (readdir(dir) == NULL)
+	if (offset < 0) {
-			return ISOBUSFS_ERR_END_OF_FILE;
+		pr_err("Invalid seek offset: %d\n", offset);
+		return ISOBUSFS_ERR_INVALID_REQUESTED_LENGHT;
+	}
+
+	/*
+	 * ISO 11783-13:2021 C.3.4.2:
+	 * Directory offsets are entry indices. If we fail to seek, restore
+	 * the previous entry position since the position shall not change on error.
+	 */
+	ret = isobusfs_srv_dir_skip_entries(handle, offset);
+	if (ret != ISOBUSFS_ERR_SUCCESS) {
+		int restore_ret;
+
+		restore_ret = isobusfs_srv_dir_skip_entries(handle, current_pos);
+		if (restore_ret != ISOBUSFS_ERR_SUCCESS) {
+			pr_warn("Failed to restore directory position after seek failure: %d -> %d",
+				current_pos, restore_ret);
+			/* Directory stream is now in undefined state.
+			 * Mark position as invalid.
+			 */
+			handle->dir_pos = -1;
+		}
+		return ret;
 	}
 
 	handle->dir_pos = offset;
@@ -796,7 +995,7 @@ static int isobusfs_srv_fa_sf_req(struct isobusfs_srv_priv *priv,
 		goto send_response;
 	}
 
-	handle = isobusfs_srv_get_handle(priv, req->handle);
+	handle = isobusfs_srv_get_client_handle(priv, client, req->handle);
 	if (!handle) {
 		pr_warn("failed to find handle: %x", req->handle);
 		error_code = ISOBUSFS_ERR_INVALID_HANDLE;

j1939acd.c

@@ -618,6 +618,12 @@ int main(int argc, char *argv[])
 			}
 			break;
 		case J1939_PGN_ADDRESS_COMMANDED:
+			/* 8 bytes NAME + 1 byte new source address */
+			if (ret < 9)
+				break;
+			/* only accept valid unicast addresses */
+			if (dat[8] > J1939_MAX_UNICAST_ADDR)
+				break;
 			if (!host_is_little_endian())
 				bswap(dat, 8);
 			memcpy(&cmd_name, dat, 8);