| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 09 Dec 2019 15:07:29 +0300
From: Vyacheslav Dubeyko <slava@...eyko.com>
To: Namjae Jeon <namjae.jeon@...sung.com>,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Cc: gregkh@...uxfoundation.org, valdis.kletnieks@...edu, hch@....de,
sj1557.seo@...sung.com
Subject: Re: [PATCH v6 07/13] exfat: add bitmap operations
On Mon, 2019-12-09 at 01:51 -0500, Namjae Jeon wrote:
> This adds the implementation of bitmap operations for exfat.
>
> Signed-off-by: Namjae Jeon <namjae.jeon@...sung.com>
> Signed-off-by: Sungjong Seo <sj1557.seo@...sung.com>
> ---
> fs/exfat/balloc.c | 272
> ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 272 insertions(+)
> create mode 100644 fs/exfat/balloc.c
>
> diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c
> new file mode 100644
> index 000000000000..d7b70620e349
> --- /dev/null
> +++ b/fs/exfat/balloc.c
> @@ -0,0 +1,272 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd.
> + */
> +
> +#include <linux/blkdev.h>
> +#include <linux/slab.h>
> +#include <linux/buffer_head.h>
> +
> +#include "exfat_raw.h"
> +#include "exfat_fs.h"
> +
> +static const unsigned char free_bit[] = {
> + 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 4, 0, 1, 0,
> 2,/* 0 ~ 19*/
> + 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 5, 0, 1, 0, 2, 0, 1, 0, 3,/*
> 20 ~ 39*/
> + 0, 1, 0, 2, 0, 1, 0, 4, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2,/*
> 40 ~ 59*/
> + 0, 1, 0, 6, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 4,/*
> 60 ~ 79*/
> + 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 5, 0, 1, 0, 2,/*
> 80 ~ 99*/
> + 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 4, 0, 1, 0, 2, 0, 1, 0,
> 3,/*100 ~ 119*/
> + 0, 1, 0, 2, 0, 1, 0, 7, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0,
> 2,/*120 ~ 139*/
> + 0, 1, 0, 4, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0,
> 5,/*140 ~ 159*/
> + 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 4, 0, 1, 0,
> 2,/*160 ~ 179*/
> + 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0, 6, 0, 1, 0, 2, 0, 1, 0,
> 3,/*180 ~ 199*/
> + 0, 1, 0, 2, 0, 1, 0, 4, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0,
> 2,/*200 ~ 219*/
> + 0, 1, 0, 5, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0,
> 4,/*220 ~ 239*/
> + 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1,
> 0 /*240 ~ 254*/
> +};
> +
> +static const unsigned char used_bit[] = {
> + 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 1, 2, 2,
> 3,/* 0 ~ 19*/
> + 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 1, 2, 2, 3, 2, 3, 3, 4,/*
> 20 ~ 39*/
> + 2, 3, 3, 4, 3, 4, 4, 5, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5,/*
> 40 ~ 59*/
> + 4, 5, 5, 6, 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5,/*
> 60 ~ 79*/
> + 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 2, 3, 3, 4,/*
> 80 ~ 99*/
> + 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 3, 4, 4, 5, 4, 5, 5,
> 6,/*100 ~ 119*/
> + 4, 5, 5, 6, 5, 6, 6, 7, 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3,
> 4,/*120 ~ 139*/
> + 3, 4, 4, 5, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5,
> 6,/*140 ~ 159*/
> + 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 3, 4, 4,
> 5,/*160 ~ 179*/
> + 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, 2, 3, 3, 4, 3, 4, 4,
> 5,/*180 ~ 199*/
> + 3, 4, 4, 5, 4, 5, 5, 6, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5,
> 6,/*200 ~ 219*/
> + 5, 6, 6, 7, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6,
> 7,/*220 ~ 239*/
> + 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7,
> 8 /*240 ~ 255*/
> +};
> +
> +/*
> + * Allocation Bitmap Management Functions
> + */
> +static int exfat_allocate_bitmap(struct super_block *sb,
> + struct exfat_dentry *ep)
> +{
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + long long map_size;
> + unsigned int i, need_map_size;
> + sector_t sector;
> +
> + sbi->map_clu = le32_to_cpu(ep->bitmap_start_clu);
> + map_size = le64_to_cpu(ep->bitmap_size);
> + need_map_size = (((sbi->num_clusters - BASE_CLUSTER) - 1) >> 3)
> + 1;
This code is slightly complicated. Why 3 was used here? Maybe it makes
sense to refactor this code and to make it more clear?
> + if (need_map_size != map_size) {
> + exfat_msg(sb, KERN_ERR,
> + "bogus allocation bitmap size(need :
> %u, cur : %lld)",
> + need_map_size, map_size);
> + /*
> + * Only allowed when bogus allocation
> + * bitmap size is large
> + */
> + if (need_map_size > map_size)
> + return -EIO;
> + }
> + sbi->map_sectors = ((need_map_size - 1) >>
> + (sb->s_blocksize_bits)) + 1;
> + sbi->vol_amap = kmalloc_array(sbi->map_sectors,
> + sizeof(struct buffer_head *),
> GFP_KERNEL);
> + if (!sbi->vol_amap)
> + return -ENOMEM;
> +
> + sector = exfat_cluster_to_sector(sbi, sbi->map_clu);
> + for (i = 0; i < sbi->map_sectors; i++) {
> + sbi->vol_amap[i] = sb_bread(sb, sector + i);
> + if (!sbi->vol_amap[i]) {
> + /* release all buffers and free vol_amap */
> + int j = 0;
> +
> + while (j < i)
> + brelse(sbi->vol_amap[j++]);
> +
> + kfree(sbi->vol_amap);
> + sbi->vol_amap = NULL;
> + return -EIO;
> + }
> + }
> +
> + sbi->pbr_bh = NULL;
> + return 0;
> +}
> +
> +int exfat_load_bitmap(struct super_block *sb)
> +{
> + unsigned int i, type;
> + struct exfat_chain clu;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + exfat_chain_set(&clu, sbi->root_dir, 0, ALLOC_FAT_CHAIN);
> +
> + while (clu.dir != EOF_CLUSTER) {
> + for (i = 0; i < sbi->dentries_per_clu; i++) {
> + struct exfat_dentry *ep;
> + struct buffer_head *bh;
> +
> + ep = exfat_get_dentry(sb, &clu, i, &bh, NULL);
> + if (!ep)
> + return -EIO;
> +
> + type = exfat_get_entry_type(ep);
> + if (type == TYPE_UNUSED)
> + break;
> + if (type != TYPE_BITMAP)
> + continue;
> + if (ep->bitmap_flags == 0x0) {
> + int err;
> +
> + err = exfat_allocate_bitmap(sb, ep);
> + brelse(bh);
> + return err;
> + }
> + brelse(bh);
> + }
> +
> + if (exfat_get_next_cluster(sb, &clu.dir))
> + return -EIO;
> + }
> +
> + return -EINVAL;
> +}
> +
> +void exfat_free_bitmap(struct super_block *sb)
> +{
> + int i;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + brelse(sbi->pbr_bh);
> +
> + for (i = 0; i < sbi->map_sectors; i++)
> + __brelse(sbi->vol_amap[i]);
> +
> + kfree(sbi->vol_amap);
> + sbi->vol_amap = NULL;
> +}
> +
> +/*
> + * If the value of "clu" is 0, it means cluster 2 which is the first
> cluster of
> + * the cluster heap.
> + */
> +int exfat_set_bitmap(struct inode *inode, unsigned int clu)
> +{
> + int i, b;
> + struct super_block *sb = inode->i_sb;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + i = clu >> (sb->s_blocksize_bits + 3);
> + b = clu & ((sb->s_blocksize << 3) - 1);
This code looks like the dark magic. I believe it makes sense to
introduce some constant for 3 here.
> +
> + set_bit_le(b, sbi->vol_amap[i]->b_data);
> + exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
> + return 0;
> +}
> +
> +/*
> + * If the value of "clu" is 0, it means cluster 2 which is the first
> cluster of
> + * the cluster heap.
> + */
> +void exfat_clear_bitmap(struct inode *inode, unsigned int clu)
> +{
> + int i, b;
> + struct super_block *sb = inode->i_sb;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + struct exfat_mount_options *opts = &sbi->options;
> +
> + i = clu >> (sb->s_blocksize_bits + 3);
> + b = clu & ((sb->s_blocksize << 3) - 1);
> +
Ditto.
> + clear_bit_le(b, sbi->vol_amap[i]->b_data);
> + exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
> +
> + if (opts->discard) {
> + int ret_discard;
> +
> + ret_discard = sb_issue_discard(sb,
> + exfat_cluster_to_sector(sbi, clu + 2),
> + (1 << sbi->sect_per_clus_bits),
> GFP_NOFS, 0);
Why clu + 2? It's unclear, frankly speaking.
> +
> + if (ret_discard == -EOPNOTSUPP) {
> + exfat_msg(sb, KERN_ERR,
> + "discard not supported by device,
> disabling");
> + opts->discard = 0;
> + }
> + }
> +}
> +
> +/*
> + * If the value of "clu" is 0, it means cluster 2 which is the first
> cluster of
> + * the cluster heap.
> + */
> +unsigned int exfat_test_bitmap(struct super_block *sb, unsigned int
> clu)
> +{
> + unsigned int i, map_i, map_b;
> + unsigned int clu_base, clu_free;
> + unsigned char k, clu_mask;
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> +
> + clu_base = (clu & ~(0x7)) + 2;
> + clu_mask = (1 << (clu - clu_base + 2)) - 1;
> +
What does 0x7 mean here? Why clu - clu_base + 2? Maybe it need to
introduce some constants and to add some comments?
> + map_i = clu >> (sb->s_blocksize_bits + 3);
> + map_b = (clu >> 3) & (unsigned int)(sb->s_blocksize - 1);
> +
Ditto.
> + for (i = 2; i < sbi->num_clusters; i += 8) {
Why we start from 2 and add 8? Maybe introduce some constants?
> + k = *(sbi->vol_amap[map_i]->b_data + map_b);
> + if (clu_mask > 0) {
> + k |= clu_mask;
> + clu_mask = 0;
> + }
> + if (k < 0xFF) {
> + clu_free = clu_base + free_bit[k];
> + if (clu_free < sbi->num_clusters)
> + return clu_free;
> + }
> + clu_base += 8;
Ditto.
> +
> + if (++map_b >= sb->s_blocksize ||
> + clu_base >= sbi->num_clusters) {
> + if (++map_i >= sbi->map_sectors) {
> + clu_base = 2;
> + map_i = 0;
> + }
> + map_b = 0;
> + }
> + }
> +
> + return EOF_CLUSTER;
> +}
> +
> +int exfat_count_used_clusters(struct super_block *sb, unsigned int
> *ret_count)
> +{
> + struct exfat_sb_info *sbi = EXFAT_SB(sb);
> + unsigned int count = 0;
> + unsigned int i, map_i = 0, map_b = 0;
> + unsigned int total_clus = sbi->num_clusters - 2;
> + unsigned int last_mask = total_clus & 7;
Ditto.
> + unsigned char clu_bits;
> + const unsigned char last_bit_mask[] = {0, 0b00000001,
> 0b00000011,
> + 0b00000111, 0b00001111, 0b00011111, 0b00111111,
> 0b01111111};
> +
> + total_clus &= ~last_mask;
> + for (i = 0; i < total_clus; i += 8) {
Ditto.
Thanks,
Viacheslav Dubeyko.
> + clu_bits = *(sbi->vol_amap[map_i]->b_data + map_b);
> + count += used_bit[clu_bits];
> + if (++map_b >= (unsigned int)sb->s_blocksize) {
> + map_i++;
> + map_b = 0;
> + }
> + }
> +
> + if (last_mask) {
> + clu_bits = *(sbi->vol_amap[map_i]->b_data + map_b);
> + clu_bits &= last_bit_mask[last_mask];
> + count += used_bit[clu_bits];
> + }
> +
> + *ret_count = count;
> + return 0;
> +}
Powered by blists - more mailing lists