On Fri, Mar 02, 2018 at 01:22:52PM +0800, Qu Wenruo wrote: > We have extra sector size check in cow_file_range_inline(), but doesn't > implement it in BTRFS_MAX_INLINE_DATA_SIZE(). > > The biggest reason is that btrfs_symlink() also uses this macro to check > name length.
I'm reading what the standard says about symlink and restrictions, http://pubs.opengroup.org/onlinepubs/009695399/functions/symlink.html the maximum length of the symlink contents should not exceed SYMLINK_MAX, 'getconf SYMLINK_MAX /path/to/btrfs' says undefined and I can't find any restrictions in manual pages. At least the target should not exceed PATH_MAX in case is's an absolute path, the rest is up to the path name resolution to decide if a relative symlink plus the base directory exceeds PATH_MAX. IOW, the symlink callback could extend the condition to check against PATH_MAX, that is independent on the page size and sectorsize. A larger symlink target is not a bug per se, just that it will never pass through VFS once it would be used for file access. > In fact such behavior makes max_inline calculation quite confusing, and > cause unexpected large extent for symbol link. > > Here we embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() so > that it will never exceed sector size. I don't think it's wise to mix sectorsize and nodesize, here the macro BTRFS_MAX_INLINE_DATA_SIZE is related only to b-tree nodes. > The downside is, for symbol link, we will reduce max symbol link length > from 16K- to 4095, but it won't affect current system using that long > name, but only prevent later creation. The page size limit will be hit so >4096 symlink targets can be created only on systems with big pages. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html