You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/30 20:56:55 UTC
[GitHub] [incubator-nuttx] vxj9800 opened a new pull request, #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
vxj9800 opened a new pull request, #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347
## Summary
After a build, nsh_romfsimg.c file contains the nodes of cromfs. These nodes are generally not word aligned. Armv6-m architecture doesn't support unaligned memory access. Thus, a hardfault is generated when these unaligned nodes are accessed from cromfs_compare_node function.
## Impact
Hardfault crashes Nuttx.
## Testing
The issue and solution are both tested on a Raspberry Pi Pico running PX4. Unfortunately, I don't know enough about Nuttx to create a defconfig file that would make use of cromfs. It will be great if a core developer can confirm this on other armv6-m MCUs running Nuttx.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#discussion_r885088784
##########
fs/cromfs/cromfs.h:
##########
@@ -116,6 +116,6 @@ struct cromfs_node_s
uint32_t cn_link; /* Offset to an arbitrary node (for hard link) */
uint32_t cn_blocks; /* Offset to first block of compressed data (for read) */
} u;
-};
+} __attribute__((packed)); /* Added to resolve hardFault caused by unaligned memory access of cromfs nodes.
Review Comment:
@vxj9800 - there is a compiler.h macros the do this in a portable way see https://github.com/apache/incubator-nuttx/commit/440ca487789fcba776fc04e90f0ce71b4c1e86d3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142486019
> @vxj9800 please squash your change into one patch.
@xiaoxiang781216 I'm sorry to ask you this, but have I done the squashing correctly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#discussion_r885999757
##########
fs/cromfs/cromfs.h:
##########
@@ -26,6 +26,7 @@
****************************************************************************/
#include <sys/types.h>
+#include <nuttx/compiler.h>
Review Comment:
```suggestion
#include <nuttx/compiler.h>
```
I think `stdint.h` already contains `include <nuttx/compiler.h>`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142414227
> > Please try if this change works fro you #6350
>
> @pkarashchenko This solution doesn't work for me. My problem is not with the alignment of g_cromfs_image. It is rather with the alignment of each node, or offset of each node, inside g_cromfs_image which is not word aligned.
Ok. Let me inspect change more carefully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on a diff in pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on code in PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#discussion_r885180891
##########
fs/cromfs/cromfs.h:
##########
@@ -116,6 +116,6 @@ struct cromfs_node_s
uint32_t cn_link; /* Offset to an arbitrary node (for hard link) */
uint32_t cn_blocks; /* Offset to first block of compressed data (for read) */
} u;
-};
+} __attribute__((packed)); /* Added to resolve hardFault caused by unaligned memory access of cromfs nodes.
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#discussion_r885490111
##########
fs/cromfs/cromfs.h:
##########
@@ -116,6 +117,6 @@ struct cromfs_node_s
uint32_t cn_link; /* Offset to an arbitrary node (for hard link) */
uint32_t cn_blocks; /* Offset to first block of compressed data (for read) */
} u;
-};
+} end_packed_struct; /* Added to resolve hardFault caused by unaligned memory access of cromfs nodes.
Review Comment:
```suggestion
} end_packed_struct; /* Added to resolve hardFault caused by unaligned memory access of cromfs nodes. */
```
to solve
```
In file included from ./cromfs/fs_cromfs.c:46:
./cromfs/cromfs.h:21: error: unterminated #ifndef
21 | #ifndef __FS_CROMFS_CROMFS_H
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142116113
> 1. Please squash changes into a single commit.
> 2. I'm wondering that this comes only in your case as NuttX have support of many platforms that does not support unaligned access. Could you please share more about your use case? Maybe in your case the memory is not allocated properly or something else. I would like to examine your case more carefully.
1. @pkarashchenko I hope I have done this correctly. I used this [answer](https://stackoverflow.com/a/5201642).
2. I was wondering that too. I am adding support for raspberry pi rp2040 to PX4, but most of the testing is done using pi pico right now. The [nsh_romfsimg](https://github.com/apache/incubator-nuttx/files/8805206/nsh_romfsimg.zip) was generated by [gencromfs.c](https://github.com/apache/incubator-nuttx/blob/3366f1fb3510a5eaca79eee2b6d831a9dbdee4c2/tools/gencromfs.c). Note that many node offsets in nsh_romfsimg.c are not word aligned. Following is the video showing the hardfault. Unfortunately, I don't know how to replicate this problem while using the Nuttx only. Please let me know if any other information is required.
https://user-images.githubusercontent.com/45529441/171179932-e3ed082e-89d9-4f95-bdc9-dec20b7bec76.mp4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142124601
Maybe you are using the older NuttX code base, because a series of changes related to ROMFS was introduced to mainline to cover similar issue:
- https://github.com/apache/incubator-nuttx/pull/5330
- https://github.com/apache/incubator-nuttx-apps/pull/986
- https://github.com/apache/incubator-nuttx/pull/5362
That is why I'm wondering that you are still reporting that problem persists.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#discussion_r886043418
##########
fs/cromfs/cromfs.h:
##########
@@ -102,20 +102,20 @@ struct cromfs_volume_s
* Return 0
*/
-struct cromfs_node_s
+begin_packed_struct struct cromfs_node_s
{
- uint16_t cn_mode; /* File type, attributes, and access mode bits */
- uint16_t cn_pad; /* Not used */
- uint32_t cn_name; /* Offset from the beginning of the volume header to the
- * node name string. NUL-terminated. */
- uint32_t cn_size; /* Size of the uncompressed data (in bytes) */
- uint32_t cn_peer; /* Offset to next node in this directory (for readdir()) */
+ uint16_t cn_mode; /* File type, attributes, and access mode bits */
+ uint16_t cn_pad; /* Not used */
+ uint32_t cn_name; /* Offset from the beginning of the volume header to the
+ * node name string. NUL-terminated. */
+ uint32_t cn_size; /* Size of the uncompressed data (in bytes) */
+ uint32_t cn_peer; /* Offset to next node in this directory (for readdir()) */
union
{
- uint32_t cn_child; /* Offset to first node in sub-directory (directories only) */
- uint32_t cn_link; /* Offset to an arbitrary node (for hard link) */
- uint32_t cn_blocks; /* Offset to first block of compressed data (for read) */
+ uint32_t cn_child; /* Offset to first node in sub-directory (directories only) */
+ uint32_t cn_link; /* Offset to an arbitrary node (for hard link) */
+ uint32_t cn_blocks; /* Offset to first block of compressed data (for read) */
} u;
-};
+} end_packed_struct; /* Added to resolve hardFault caused by unaligned memory access of cromfs nodes. */
Review Comment:
```suggestion
} end_packed_struct; /* Use packed access since cromfs nodes may be unaligned */
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142129411
Oah.. I see now. It is not romfs, but cromfs. Then maybe it is better to update the tool and add 4 byte alignment attribute as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142155233
Please try if this change works fro you https://github.com/apache/incubator-nuttx/pull/6350
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142511093
> @vxj9800 I re-examined the `gencromfs.c` and really do not see how node alignment can be done easily so we need to go with `packed` `struct cromfs_node_s` at least for now.
@pkarashchenko I agree to that. And I don't think that the nodes in g_cromfs_image need to be word aligned since that will somewhat defeat the purpose of compression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142515725
Anyway let's merge my change as well, just to ensure that cromfs header is word aligned and accessing "magic" and other fields will not lead to crash
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142537718
I tried squashing the commits again. I'm really not sure what I'm doing wrong. Here is an image
.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142542350
> I tried squashing the commits again. I'm really not sure what I'm doing wrong. Here is an image  .
I think you are doing everything right. You just need to `git push -f origin cromfs-unaligned-mem-access` after you finish rebase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] vxj9800 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
vxj9800 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142209194
> Please try if this change works fro you #6350
@pkarashchenko This solution doesn't work for me. My problem is not with the alignment of g_cromfs_image. It is rather with the alignment of each node, or offset of each node, inside g_cromfs_image which is not word aligned.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1141700947
1. Please squash changes into a single commit.
2. I'm wondering that this comes only in your case as NuttX have support of many platforms that does not support unaligned access. Could you please share more about your use case? Maybe in your case the memory is not allocated properly or something else. I would like to examine your case more carefully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142433451
@vxj9800 please squash your change into one patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142500136
> > @vxj9800 please squash your change into one patch.
>
> @xiaoxiang781216 I'm sorry to ask you this, but have I done the squashing correctly?
No, when you do the `rebase -i` you need to mark only one commit as `pick` and all other as `squash`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6347: fs/cromfs: Fixed a hardFault caused by unaligned memory access
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6347:
URL: https://github.com/apache/incubator-nuttx/pull/6347#issuecomment-1142507852
@vxj9800 I re-examined the `gencromfs.c` and really do not see how node alignment can be done easily so we need to go with `packed` `struct cromfs_node_s` at least for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org