20.11.2015 00:28, Jaegeuk Kim пишет:
> Hello,
>
> Change log from v2:
> o Enhance the code quality suggested by Andrei
>
> Sorry for the long delay.
> Could you please check this patch?
>
> Thank you so much,
>
Thank you for continuing to work on it!
...
> +
> +static inline int
> +grub_generic_test_bit (int nr, const grub_uint32_t *addr)
> +{
> + return 1UL & (addr[nr / 32] >> (nr & 31));
> +}
> +
As already discussed this code is wrong on big-endian platform. On-disk
bitmap is little-endian and kernel explicitly uses test_bit_le() here.
> +static inline char *
> +__inline_addr (struct grub_f2fs_inode *inode)
> +{
> + return (char *)&inode->i_addr[1];
> +}
> +
> +static inline grub_uint64_t
> +grub_f2fs_file_size (struct grub_f2fs_inode *inode)
> +{
> + return grub_le_to_cpu64 (inode->i_size);
> +}
> +
> +static inline grub_uint32_t
> +__start_cp_addr (struct grub_f2fs_data *data)
> +{
> + struct grub_f2fs_checkpoint *ckpt = &data->ckpt;
> + grub_uint64_t ckpt_version = grub_le_to_cpu64 (ckpt->checkpoint_ver);
> + grub_uint32_t start_addr = data->cp_blkaddr;
> +
> + if (!(ckpt_version & 1))
This can use grub_cpu_to_le64_compile_time (1)
...
> +static inline int
> +grub_f2fs_test_bit (grub_uint32_t nr, const char *p)
> +{
> + int mask;
> +
> + p += (nr >> 3);
> + mask = 1 << (7 - (nr & 0x07));
> + return (mask & *p) != 0;
This is really just "return mask & *p".
> +}
> +
> +static int
> +grub_f2fs_sanity_check_sb (struct grub_f2fs_superblock *sb)
> +{
> + grub_uint32_t log_sectorsize, log_sectors_per_block;
> +
> + if (sb->magic != grub_cpu_to_le32_compile_time (F2FS_SUPER_MAGIC))
> + return -1;
> +
> + if (sb->log_blocksize != grub_cpu_to_le32_compile_time (F2FS_BLK_BITS))
> + return -1;
> +
> + log_sectorsize = grub_le_to_cpu32 (sb->log_sectorsize);
> + log_sectors_per_block = grub_le_to_cpu32 (sb->log_sectors_per_block);
> +
> + if (log_sectorsize > F2FS_MAX_LOG_SECTOR_SIZE)
> + return -1;
> +
> + if (log_sectorsize < F2FS_MIN_LOG_SECTOR_SIZE)
> + return -1;
> +
> + if (log_sectors_per_block + log_sectorsize != F2FS_MAX_LOG_SECTOR_SIZE)
This sounds like it should actually be F2FS_BLK_BITS; at least assuming
that F2FS_MAX_LOG_SECTOR_SIZE may differ from F2FS_BLK_BITS.
...
> +
> +static grub_ssize_t
> +grub_f2fs_read_file (grub_fshelp_node_t node,
> + grub_disk_read_hook_t read_hook, void *read_hook_data,
> + grub_off_t pos, grub_size_t len, char *buf)
> +{
> + struct grub_f2fs_inode *inode = &node->inode.i;
> + grub_off_t filesize = grub_f2fs_file_size (inode);
> + char *inline_addr = __inline_addr (inode);
> +
> + if (inode->i_inline & F2FS_INLINE_DATA)
> + {
> + if (pos > filesize || filesize > MAX_INLINE_DATA)
> + {
> + grub_error (GRUB_ERR_BAD_FS, "corrupted inline_data: need fsck");
Sorry for confusion, my fault. pos > filesize was OK, but filesize >
MAX_INLINE_DATA not.
...
> +
> +/* TODO: mkfs.f2fs stores label in a wrong way. Should be fixed. */
> +static void
> +grub_f2fs_unicode_to_ascii (grub_uint8_t *out_buf, grub_uint16_t *in_buf)
> +{
> + grub_uint16_t *pchTempPtr = in_buf;
> + grub_uint8_t *pwTempPtr = out_buf;
> +
> + while (*pchTempPtr != '\0')
> + {
> + *pwTempPtr = (grub_uint8_t) *pchTempPtr;
> + pchTempPtr++;
> + pwTempPtr++;
> + }
> + *pwTempPtr = '\0';
> + return;
> +}
Sorry, I do not see how it can work on both big and little endian
platforms. What byte order is used for on-disk label? Why cannot you use
grub_utf16_to_utf8 as I asked last time?
Also please add bundary check, do not rely on correct content.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
