* [PATCH] Ignore symlink traversal failures in grub-mount readdir @ 2012-10-12 16:09 Colin Watson 2012-10-15 15:08 ` Dave Vasilevsky 2013-03-09 16:47 ` Andrey Borzenkov 0 siblings, 2 replies; 6+ messages in thread From: Colin Watson @ 2012-10-12 16:09 UTC (permalink / raw) To: grub-devel This is very much a temporary hack, so I'm sending it here for discussion rather than just committing it even though it's quite simple. r3036.1.15 introduced support for filling in the attributes of files in fuse_readdir. However, symlinks to directories are passed to call_fill with info.dir unset; call_fill will then try to open them with grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a directory not a regular file. It then causes the whole readdir call to fail. The net effect is that if you, for example, have a symlink anywhere in the top level of a filesystem, then the entire filesystem appears empty in grub-mount. This is the root cause of https://bugs.launchpad.net/bugs/1051306. I think that the proper solution is to pass the full grub_fshelp_filetype to dirhook functions, which would permit implementing true symlink support in grub-mount. That would be a fairly large change that I don't have time for at the moment. As a stopgap, I suggest that we ignore errors from individual grub_file_open calls during fuse_readdir. How does this patch look? 2012-10-12 Colin Watson <cjwatson@ubuntu.com> === modified file 'util/grub-mount.c' --- util/grub-mount.c 2012-03-08 18:09:05 +0000 +++ util/grub-mount.c 2012-10-12 16:08:39 +0000 @@ -292,7 +292,20 @@ fuse_readdir (const char *path, void *bu file = grub_file_open (tmp); free (tmp); if (! file) - return translate_error (); + { + /* We cannot handle symlinks properly yet, and symlinks to + directories will cause us to reach here. Symlink loops or + dangling symlinks will also cause an error. For the + meantime, while treating these as zero-length files is wrong, + it's better than failing the whole readdir call by returning + translate_error (). + + Ultimately, we should be able to tell from the + grub_dirhook_info that this is a symlink, and fill in the + attributes of the symlink rather than its target. */ + grub_errno = GRUB_ERR_NONE; + return 0; + } st.st_size = file->size; grub_file_close (file); } Thanks, -- Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir 2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson @ 2012-10-15 15:08 ` Dave Vasilevsky 2013-03-09 16:47 ` Andrey Borzenkov 1 sibling, 0 replies; 6+ messages in thread From: Dave Vasilevsky @ 2012-10-15 15:08 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: cjwatson > call_fill will then try to open them with > grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a > directory not a regular file. It then causes the whole readdir call to > fail. The net effect is that if you, for example, have a symlink > anywhere in the top level of a filesystem, then the entire filesystem > appears empty in grub-mount. I used this extremely kludgy fix: http://bazaar.launchpad.net/~djvasi/grub/mac-grub-mount/revision/4246#util/grub-mount.c . It just assumes that if we get GRUB_ERR_BAD_FILE_TYPE, it must have been a symlink to a directory, and presents it as such. Obviously this is also a temporary hack, not sure if it's any better or worse than yours. Cheers, Dave Vasilevsky ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir 2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson 2012-10-15 15:08 ` Dave Vasilevsky @ 2013-03-09 16:47 ` Andrey Borzenkov 2013-03-10 13:01 ` Vladimir 'φ-coder/phcoder' Serbinenko 1 sibling, 1 reply; 6+ messages in thread From: Andrey Borzenkov @ 2013-03-09 16:47 UTC (permalink / raw) To: grub-devel В Fri, 12 Oct 2012 17:09:54 +0100 Colin Watson <cjwatson@ubuntu.com> пишет: > This is very much a temporary hack, so I'm sending it here for > discussion rather than just committing it even though it's quite simple. > > r3036.1.15 introduced support for filling in the attributes of files in > fuse_readdir. However, symlinks to directories are passed to call_fill > with info.dir unset; call_fill will then try to open them with > grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a > directory not a regular file. It then causes the whole readdir call to > fail. The net effect is that if you, for example, have a symlink > anywhere in the top level of a filesystem, then the entire filesystem > appears empty in grub-mount. This is the root cause of > https://bugs.launchpad.net/bugs/1051306. > > I think that the proper solution is to pass the full > grub_fshelp_filetype to dirhook functions, which would permit > implementing true symlink support in grub-mount. That would be a fairly > large change that I don't have time for at the moment. As a stopgap, I > suggest that we ignore errors from individual grub_file_open calls > during fuse_readdir. How does this patch look? > The only reason to call grub_file_open() is to fetch file size, and file size is already available when hooks are called. So what about patch below? It fixes problem for me, and it trivial enough. This allows directory listing to work again. I can extend it with info.is_link to return proper filetype to FUSE, but implementing full support needs adding readlink that is a separate topic. I tested it with ext4 and cpio and it works. Testing on more systems (in particular, NTFS, which is the only one with non-trivial change) is appreciated. --- From: Andrey Borzenkov <arvidjaar@gmail.com> Subject: [PATCH] return file size in grub_dirhook_info for FUSE The only reason for using grub_file_open() in fuse_getattr() and fuse_readdir_call_fill() was to get file size. But file size is already available at the place when these hooks are called in all filesystems except NTFS. So the call is redundant; instead simply return file size as part of struct grub_dirhook_info. For NTFS this patch additionally adds reading "inode" information from disk, as it was not available otherwise. All additional code is under #ifdef GRUB_UTLI to not impact boot time sizes. This fixes problem with listing directories containing symlinks (http://lists.gnu.org/archive/html/grub-devel/2012-10/msg00021.html). Attempt to access these symlinks still fails of course, full support for symlinks requires more efforts. Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com> --- grub-core/fs/affs.c | 3 +++ grub-core/fs/bfs.c | 3 +++ grub-core/fs/btrfs.c | 3 +++ grub-core/fs/cpio.c | 3 +++ grub-core/fs/ext2.c | 4 ++++ grub-core/fs/fat.c | 3 +++ grub-core/fs/hfs.c | 3 +++ grub-core/fs/hfsplus.c | 3 +++ grub-core/fs/iso9660.c | 3 +++ grub-core/fs/jfs.c | 3 +++ grub-core/fs/minix.c | 3 +++ grub-core/fs/nilfs2.c | 3 +++ grub-core/fs/ntfs.c | 7 +++++++ grub-core/fs/reiserfs.c | 3 +++ grub-core/fs/romfs.c | 3 +++ grub-core/fs/sfs.c | 3 +++ grub-core/fs/squash4.c | 11 +++++++++++ grub-core/fs/udf.c | 3 +++ grub-core/fs/ufs.c | 3 +++ grub-core/fs/xfs.c | 3 +++ grub-core/fs/zfs/zfs.c | 3 +++ include/grub/fs.h | 3 +++ util/grub-mount.c | 25 ++----------------------- 23 files changed, 81 insertions(+), 23 deletions(-) diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c index 726704e..fead360 100644 --- a/grub-core/fs/affs.c +++ b/grub-core/fs/affs.c @@ -565,6 +565,9 @@ grub_affs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = aftime2ctime (&node->di.mtime); +#ifdef GRUB_UTIL + info.size = grub_be_to_cpu32 (node->di.size); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/bfs.c b/grub-core/fs/bfs.c index f2e39d3..3b8357b 100644 --- a/grub-core/fs/bfs.c +++ b/grub-core/fs/bfs.c @@ -891,6 +891,9 @@ grub_bfs_dir_iter (const char *name, grub_uint64_t value, info.mtime = grub_bfs_to_cpu64 (ino.ino.mtime) >> 16; #endif info.dir = ((grub_bfs_to_cpu32 (ino.ino.mode) & ATTR_TYPE) == ATTR_DIR); +#ifdef GRUB_UTIL + info.size = grub_bfs_to_cpu64 (ino.ino.size); +#endif return ctx->hook (name, &info, ctx->hook_data); } diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c index 196f301..2b169aa 100644 --- a/grub-core/fs/btrfs.c +++ b/grub-core/fs/btrfs.c @@ -1581,6 +1581,9 @@ grub_btrfs_dir (grub_device_t device, const char *path, { info.mtime = grub_le_to_cpu64 (inode.mtime.sec); info.mtimeset = 1; +#ifdef GRUB_UTIL + info.size = grub_le_to_cpu64 (inode.size); +#endif } c = cdirel->name[grub_le_to_cpu16 (cdirel->n)]; cdirel->name[grub_le_to_cpu16 (cdirel->n)] = 0; diff --git a/grub-core/fs/cpio.c b/grub-core/fs/cpio.c index 71d7fa4..c6885d4 100644 --- a/grub-core/fs/cpio.c +++ b/grub-core/fs/cpio.c @@ -573,6 +573,9 @@ grub_cpio_dir (grub_device_t device, const char *path_in, info.dir = (p != NULL) || ((mode & ATTR_TYPE) == ATTR_DIR); info.mtime = mtime; info.mtimeset = 1; +#ifdef GRUB_UTIL + info.size = data->size; +#endif if (hook (n, &info, hook_data)) { diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c index 18429ac..215d1d2 100644 --- a/grub-core/fs/ext2.c +++ b/grub-core/fs/ext2.c @@ -884,6 +884,10 @@ grub_ext2_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, { info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (node->inode.mtime); +#ifdef GRUB_UTIL + info.size = grub_cpu_to_le32 (node->inode.size) | + (((grub_off_t) grub_cpu_to_le32 (node->inode.size_high)) << 32); +#endif } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c index db28158..f6cb0cd 100644 --- a/grub-core/fs/fat.c +++ b/grub-core/fs/fat.c @@ -894,6 +894,9 @@ grub_fat_find_dir (grub_disk_t disk, struct grub_fat_data *data, info.dir = !! (ctxt.dir.attr & GRUB_FAT_ATTR_DIRECTORY); info.case_insensitive = 1; +#ifdef GRUB_UTIL + info.size = data->file_size; +#endif #ifdef MODE_EXFAT if (!ctxt.dir.have_stream) diff --git a/grub-core/fs/hfs.c b/grub-core/fs/hfs.c index 73ac7f9..39fec63 100644 --- a/grub-core/fs/hfs.c +++ b/grub-core/fs/hfs.c @@ -1225,6 +1225,9 @@ grub_hfs_dir_hook (struct grub_hfs_record *rec, void *hook_arg) info.dir = 0; info.mtimeset = 1; info.mtime = grub_be_to_cpu32 (frec->mtime) - 2082844800; +#ifdef GRUB_UTIL + info.size = grub_be_to_cpu32 (frec->size); +#endif return ctx->hook (fname, &info, ctx->hook_data); } diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index a507c0f..47e4e58 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -1013,6 +1013,9 @@ grub_hfsplus_dir_iter (const char *filename, info.mtimeset = 1; info.mtime = node->mtime; info.case_insensitive = !! (filetype & GRUB_FSHELP_CASE_INSENSITIVE); +#ifdef GRUB_UTIL + info.size = node->size; +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c index cdbd6dc..291ee00 100644 --- a/grub-core/fs/iso9660.c +++ b/grub-core/fs/iso9660.c @@ -861,6 +861,9 @@ grub_iso9660_dir_iter (const char *filename, grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = !!iso9660_to_unixtime2 (&node->dirents[0].mtime, &info.mtime); +#ifdef GRUB_UTIL + info.size = get_node_size (node); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c index 4122eff..f800c50 100644 --- a/grub-core/fs/jfs.c +++ b/grub-core/fs/jfs.c @@ -832,6 +832,9 @@ grub_jfs_dir (grub_device_t device, const char *path, & GRUB_JFS_FILETYPE_MASK) == GRUB_JFS_FILETYPE_DIR; info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (inode.mtime.sec); +#ifdef GRUB_UTIL + info.size = grub_le_to_cpu64 (inode.size); +#endif if (hook (diro->name, &info, hook_data)) goto fail; } diff --git a/grub-core/fs/minix.c b/grub-core/fs/minix.c index 225770a..1678adc 100644 --- a/grub-core/fs/minix.c +++ b/grub-core/fs/minix.c @@ -587,6 +587,9 @@ grub_minix_dir (grub_device_t device, const char *path, & GRUB_MINIX_IFDIR) == GRUB_MINIX_IFDIR); info.mtimeset = 1; info.mtime = grub_minix_to_cpu32 (data->inode.mtime); +#ifdef GRUB_UTIL + info.size = GRUB_MINIX_INODE_SIZE (data); +#endif if (hook (filename, &info, hook_data) ? 1 : 0) break; diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c index 3f28bd7..525690f 100644 --- a/grub-core/fs/nilfs2.c +++ b/grub-core/fs/nilfs2.c @@ -1056,6 +1056,9 @@ grub_nilfs2_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, { info.mtimeset = 1; info.mtime = grub_le_to_cpu64 (node->inode.i_mtime); +#ifdef GRUB_UTIL + info.size = grub_le_to_cpu64 (node->inode.i_size); +#endif } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index 5ea2e1b..5850ef2 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -1023,6 +1023,13 @@ grub_ntfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.mtime = grub_divmod64 (node->mtime, 10000000, 0) - 86400ULL * 365 * (1970 - 1601) - 86400ULL * ((1970 - 1601) / 4) + 86400ULL * ((1970 - 1601) / 100); +#ifdef GRUB_UTIL + if (!node->inode_read) + init_file (node, node->ino); + if (node->inode_read) + info.size = node->size; + free_file (node); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/reiserfs.c b/grub-core/fs/reiserfs.c index eaa7ade..6aef49b 100644 --- a/grub-core/fs/reiserfs.c +++ b/grub-core/fs/reiserfs.c @@ -1271,6 +1271,9 @@ grub_reiserfs_dir_iter (const char *filename, info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = node->mtime; +#ifdef GRUB_UTIL + info.size = node->size; +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/romfs.c b/grub-core/fs/romfs.c index 2e35444..c140d25 100644 --- a/grub-core/fs/romfs.c +++ b/grub-core/fs/romfs.c @@ -331,6 +331,9 @@ grub_romfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); +#ifdef GRUB_UTIL + info.size = grub_be_to_cpu32 (node->file.size); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/sfs.c b/grub-core/fs/sfs.c index e7d2f72..67fdee6 100644 --- a/grub-core/fs/sfs.c +++ b/grub-core/fs/sfs.c @@ -671,6 +671,9 @@ grub_sfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtime = node->mtime + 8 * 365 * 86400 + 86400 * 2; info.mtimeset = 1; +#ifdef GRUB_UTIL + info.size = node->size; +#endif grub_free (node->cache); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); diff --git a/grub-core/fs/squash4.c b/grub-core/fs/squash4.c index cb3cc3a..4eea4b2 100644 --- a/grub-core/fs/squash4.c +++ b/grub-core/fs/squash4.c @@ -656,6 +656,17 @@ grub_squash_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (node->ino.mtime); +#ifdef GRUB_UTIL + switch (node->ino.type) + { + case grub_cpu_to_le16_compile_time (SQUASH_TYPE_LONG_REGULAR): + info.size = grub_le_to_cpu64 (node->ino.long_file.size); + break; + case grub_cpu_to_le16_compile_time (SQUASH_TYPE_REGULAR): + info.size = grub_le_to_cpu32 (node->ino.file.size); + break; + } +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c index 405935e..e2d45aa 100644 --- a/grub-core/fs/udf.c +++ b/grub-core/fs/udf.c @@ -1053,6 +1053,9 @@ grub_udf_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.mtime -= 60 * tz; } +#ifdef GRUB_UTIL + info.size = U64 (node->block.fe.file_size); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/ufs.c b/grub-core/fs/ufs.c index c155912..956bfee 100644 --- a/grub-core/fs/ufs.c +++ b/grub-core/fs/ufs.c @@ -695,6 +695,9 @@ grub_ufs_dir (grub_device_t device, const char *path, info.mtime = grub_ufs_to_cpu32 (inode.mtime); #endif info.mtimeset = 1; +#ifdef GRUB_UTIL + info.size = grub_ufs_to_cpu64 (inode.size); +#endif if (hook (filename, &info, hook_data)) break; diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c index a5a1700..40851a8 100644 --- a/grub-core/fs/xfs.c +++ b/grub-core/fs/xfs.c @@ -733,6 +733,9 @@ grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype, info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec); } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); +#ifdef GRUB_UTIL + info.size = grub_be_to_cpu64 (node->inode.size); +#endif grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c index 822d65b..5635fca 100644 --- a/grub-core/fs/zfs/zfs.c +++ b/grub-core/fs/zfs/zfs.c @@ -3821,6 +3821,9 @@ iterate_zap (const char *name, grub_uint64_t val, struct grub_zfs_dir_ctx *ctx) info.mtimeset = 1; info.mtime = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_MTIME_OFFSET), dn.endian); info.case_insensitive = ctx->data->subvol.case_insensitive; +#ifdef GRUB_UTIL + info.size = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_SIZE_OFFSET), dn.endian); +#endif } if (dn.dn.dn_bonustype == DMU_OT_ZNODE) diff --git a/include/grub/fs.h b/include/grub/fs.h index e451797..e6663d8 100644 --- a/include/grub/fs.h +++ b/include/grub/fs.h @@ -39,6 +39,9 @@ struct grub_dirhook_info unsigned mtimeset:1; unsigned case_insensitive:1; grub_int32_t mtime; +#ifdef GRUB_UTIL + grub_off_t size; +#endif }; typedef int (*grub_fs_dir_hook_t) (const char *filename, diff --git a/util/grub-mount.c b/util/grub-mount.c index d0ab6a2..5a055ab 100644 --- a/util/grub-mount.c +++ b/util/grub-mount.c @@ -205,17 +205,7 @@ fuse_getattr (const char *path, struct stat *st) st->st_uid = 0; st->st_gid = 0; st->st_rdev = 0; - if (!ctx.file_info.dir) - { - grub_file_t file; - file = grub_file_open (path); - if (! file) - return translate_error (); - st->st_size = file->size; - grub_file_close (file); - } - else - st->st_size = 0; + st->st_size = ctx.file_info.size; st->st_blksize = 512; st->st_blocks = (st->st_size + 511) >> 9; st->st_atime = st->st_mtime = st->st_ctime = ctx.file_info.mtimeset @@ -297,18 +287,7 @@ fuse_readdir_call_fill (const char *filename, grub_memset (&st, 0, sizeof (st)); st.st_mode = info->dir ? (0555 | S_IFDIR) : (0444 | S_IFREG); - if (!info->dir) - { - grub_file_t file; - char *tmp; - tmp = xasprintf ("%s/%s", ctx->path, filename); - file = grub_file_open (tmp); - free (tmp); - if (! file) - return translate_error (); - st.st_size = file->size; - grub_file_close (file); - } + st.st_size = info->size; st.st_blksize = 512; st.st_blocks = (st.st_size + 511) >> 9; st.st_atime = st.st_mtime = st.st_ctime -- tg: (475ef5e..) symlink/file-size (depends on: master) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir 2013-03-09 16:47 ` Andrey Borzenkov @ 2013-03-10 13:01 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-03-10 13:02 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-11-28 13:30 ` Andrey Borzenkov 0 siblings, 2 replies; 6+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-03-10 13:01 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 2249 bytes --] On 09.03.2013 17:47, Andrey Borzenkov wrote: > В Fri, 12 Oct 2012 17:09:54 +0100 > Colin Watson <cjwatson@ubuntu.com> пишет: > >> This is very much a temporary hack, so I'm sending it here for >> discussion rather than just committing it even though it's quite simple. >> >> r3036.1.15 introduced support for filling in the attributes of files in >> fuse_readdir. However, symlinks to directories are passed to call_fill >> with info.dir unset; call_fill will then try to open them with >> grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a >> directory not a regular file. It then causes the whole readdir call to >> fail. The net effect is that if you, for example, have a symlink >> anywhere in the top level of a filesystem, then the entire filesystem >> appears empty in grub-mount. This is the root cause of >> https://bugs.launchpad.net/bugs/1051306. >> >> I think that the proper solution is to pass the full >> grub_fshelp_filetype to dirhook functions, which would permit >> implementing true symlink support in grub-mount. That would be a fairly >> large change that I don't have time for at the moment. As a stopgap, I >> suggest that we ignore errors from individual grub_file_open calls >> during fuse_readdir. How does this patch look? >> > > The only reason to call grub_file_open() is to fetch file size, and file > size is already available when hooks are called. So what about patch > below? It fixes problem for me, and it trivial enough. This allows > directory listing to work again. I can extend it with info.is_link to > return proper filetype to FUSE, but implementing full support needs > adding readlink that is a separate topic. > > I tested it with ext4 and cpio and it works. Testing on more systems > (in particular, NTFS, which is the only one with non-trivial change) is > appreciated. > Some time ago I made a similar patch but for another motivation: current code is way too inefficient for large directories as you have to rescan directory for every file. The problem with this patch is 10 bytes increase of core.img. This may be acceptable given this problem (it happens in ls on runtime as well) and inefficency of scanning. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 294 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir 2013-03-10 13:01 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-03-10 13:02 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-11-28 13:30 ` Andrey Borzenkov 1 sibling, 0 replies; 6+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-03-10 13:02 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1.1: Type: text/plain, Size: 2372 bytes --] On 10.03.2013 14:01, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > On 09.03.2013 17:47, Andrey Borzenkov wrote: > >> В Fri, 12 Oct 2012 17:09:54 +0100 >> Colin Watson <cjwatson@ubuntu.com> пишет: >> >>> This is very much a temporary hack, so I'm sending it here for >>> discussion rather than just committing it even though it's quite simple. >>> >>> r3036.1.15 introduced support for filling in the attributes of files in >>> fuse_readdir. However, symlinks to directories are passed to call_fill >>> with info.dir unset; call_fill will then try to open them with >>> grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a >>> directory not a regular file. It then causes the whole readdir call to >>> fail. The net effect is that if you, for example, have a symlink >>> anywhere in the top level of a filesystem, then the entire filesystem >>> appears empty in grub-mount. This is the root cause of >>> https://bugs.launchpad.net/bugs/1051306. >>> >>> I think that the proper solution is to pass the full >>> grub_fshelp_filetype to dirhook functions, which would permit >>> implementing true symlink support in grub-mount. That would be a fairly >>> large change that I don't have time for at the moment. As a stopgap, I >>> suggest that we ignore errors from individual grub_file_open calls >>> during fuse_readdir. How does this patch look? >>> >> >> The only reason to call grub_file_open() is to fetch file size, and file >> size is already available when hooks are called. So what about patch >> below? It fixes problem for me, and it trivial enough. This allows >> directory listing to work again. I can extend it with info.is_link to >> return proper filetype to FUSE, but implementing full support needs >> adding readlink that is a separate topic. >> >> I tested it with ext4 and cpio and it works. Testing on more systems >> (in particular, NTFS, which is the only one with non-trivial change) is >> appreciated. >> > > Some time ago I made a similar patch but for another motivation: current > code is way too inefficient for large directories as you have to rescan > directory for every file. The problem with this patch is 10 bytes > increase of core.img. This may be acceptable given this problem (it > happens in ls on runtime as well) and inefficency of scanning. > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.2: size.diff --] [-- Type: text/x-diff; name="size.diff", Size: 12849 bytes --] === modified file 'grub-core/commands/ls.c' --- grub-core/commands/ls.c 2013-01-21 16:46:24 +0000 +++ grub-core/commands/ls.c 2013-03-10 00:21:42 +0000 @@ -118,34 +118,12 @@ if (! info->dir) { - grub_file_t file; - char *pathname; - - if (ctx->dirname[grub_strlen (ctx->dirname) - 1] == '/') - pathname = grub_xasprintf ("%s%s", ctx->dirname, filename); - else - pathname = grub_xasprintf ("%s/%s", ctx->dirname, filename); - - if (!pathname) - return 1; - - /* XXX: For ext2fs symlinks are detected as files while they - should be reported as directories. */ - grub_file_filter_disable_compression (); - file = grub_file_open (pathname); - if (! file) - { - grub_errno = 0; - grub_free (pathname); - return 0; - } - if (! ctx->human) - grub_printf ("%-12llu", (unsigned long long) file->size); + grub_printf ("%-12llu", (unsigned long long) info->size); else { - grub_uint64_t fsize = file->size * 100ULL; - grub_uint64_t fsz = file->size; + grub_uint64_t fsize = info->size * 100ULL; + grub_uint64_t fsz = info->size; int units = 0; char buf[20]; @@ -168,11 +146,9 @@ grub_printf ("%-12s", buf); } else - grub_printf ("%-12llu", (unsigned long long) file->size); + grub_printf ("%-12llu", (unsigned long long) info->size); } - grub_file_close (file); - grub_free (pathname); } else grub_printf ("%-12s", _("DIR")); === modified file 'grub-core/fs/affs.c' --- grub-core/fs/affs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/affs.c 2013-03-10 00:21:07 +0000 @@ -565,6 +565,7 @@ info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = aftime2ctime (&node->di.mtime); + info.size = grub_be_to_cpu32 (node->di.size); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } === modified file 'grub-core/fs/bfs.c' --- grub-core/fs/bfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/bfs.c 2013-03-10 00:11:14 +0000 @@ -890,6 +890,7 @@ #else info.mtime = grub_bfs_to_cpu64 (ino.ino.mtime) >> 16; #endif + info.size = grub_bfs_to_cpu64 (ino.ino.size); info.dir = ((grub_bfs_to_cpu32 (ino.ino.mode) & ATTR_TYPE) == ATTR_DIR); return ctx->hook (name, &info, ctx->hook_data); } === modified file 'grub-core/fs/btrfs.c' --- grub-core/fs/btrfs.c 2013-01-21 01:33:46 +0000 +++ grub-core/fs/btrfs.c 2013-03-10 00:05:22 +0000 @@ -1581,6 +1581,7 @@ { info.mtime = grub_le_to_cpu64 (inode.mtime.sec); info.mtimeset = 1; + info.size = grub_le_to_cpu64 (inode.size); } c = cdirel->name[grub_le_to_cpu16 (cdirel->n)]; cdirel->name[grub_le_to_cpu16 (cdirel->n)] = 0; === modified file 'grub-core/fs/cpio.c' --- grub-core/fs/cpio.c 2013-01-21 01:33:46 +0000 +++ grub-core/fs/cpio.c 2013-03-10 00:05:22 +0000 @@ -573,6 +573,7 @@ info.dir = (p != NULL) || ((mode & ATTR_TYPE) == ATTR_DIR); info.mtime = mtime; info.mtimeset = 1; + info.size = data->size; if (hook (n, &info, hook_data)) { === modified file 'grub-core/fs/ext2.c' --- grub-core/fs/ext2.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/ext2.c 2013-03-10 12:08:40 +0000 @@ -884,6 +884,8 @@ { info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (node->inode.mtime); + info.size = grub_le_to_cpu32 (node->inode.size) + | (((grub_off_t) grub_le_to_cpu32 (node->inode.size_high)) << 32); } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); === modified file 'grub-core/fs/fat.c' --- grub-core/fs/fat.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/fat.c 2013-03-10 12:46:55 +0000 @@ -894,6 +894,7 @@ info.dir = !! (ctxt.dir.attr & GRUB_FAT_ATTR_DIRECTORY); info.case_insensitive = 1; + info.size = ctxt.dir.file_size; #ifdef MODE_EXFAT if (!ctxt.dir.have_stream) === modified file 'grub-core/fs/hfs.c' --- grub-core/fs/hfs.c 2013-03-02 10:31:00 +0000 +++ grub-core/fs/hfs.c 2013-03-10 00:13:16 +0000 @@ -1225,6 +1225,7 @@ info.dir = 0; info.mtimeset = 1; info.mtime = grub_be_to_cpu32 (frec->mtime) - 2082844800; + info.size = grub_be_to_cpu32 (frec->size); return ctx->hook (fname, &info, ctx->hook_data); } === modified file 'grub-core/fs/hfsplus.c' --- grub-core/fs/hfsplus.c 2013-03-01 13:02:27 +0000 +++ grub-core/fs/hfsplus.c 2013-03-10 00:13:52 +0000 @@ -1012,6 +1012,7 @@ info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = node->mtime; + info.size = node->size; info.case_insensitive = !! (filetype & GRUB_FSHELP_CASE_INSENSITIVE); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); === modified file 'grub-core/fs/iso9660.c' --- grub-core/fs/iso9660.c 2013-03-07 08:11:36 +0000 +++ grub-core/fs/iso9660.c 2013-03-10 00:14:17 +0000 @@ -861,6 +861,7 @@ grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = !!iso9660_to_unixtime2 (&node->dirents[0].mtime, &info.mtime); + info.size = get_node_size (node); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); === modified file 'grub-core/fs/jfs.c' --- grub-core/fs/jfs.c 2013-02-28 09:51:32 +0000 +++ grub-core/fs/jfs.c 2013-03-10 00:14:41 +0000 @@ -832,6 +832,7 @@ & GRUB_JFS_FILETYPE_MASK) == GRUB_JFS_FILETYPE_DIR; info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (inode.mtime.sec); + info.size = grub_le_to_cpu64 (inode.size); if (hook (diro->name, &info, hook_data)) goto fail; } === modified file 'grub-core/fs/minix.c' --- grub-core/fs/minix.c 2013-02-28 09:50:01 +0000 +++ grub-core/fs/minix.c 2013-03-10 00:05:22 +0000 @@ -587,6 +587,7 @@ & GRUB_MINIX_IFDIR) == GRUB_MINIX_IFDIR); info.mtimeset = 1; info.mtime = grub_minix_to_cpu32 (data->inode.mtime); + info.size = GRUB_MINIX_INODE_SIZE (data); if (hook (filename, &info, hook_data) ? 1 : 0) break; === modified file 'grub-core/fs/nilfs2.c' --- grub-core/fs/nilfs2.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/nilfs2.c 2013-03-10 00:15:05 +0000 @@ -1056,6 +1056,7 @@ { info.mtimeset = 1; info.mtime = grub_le_to_cpu64 (node->inode.i_mtime); + info.size = grub_le_to_cpu64 (node->inode.i_size); } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); === modified file 'grub-core/fs/ntfs.c' --- grub-core/fs/ntfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/ntfs.c 2013-03-10 00:15:49 +0000 @@ -1019,10 +1019,13 @@ grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); + init_file (node, node->ino); info.mtimeset = 1; info.mtime = grub_divmod64 (node->mtime, 10000000, 0) - 86400ULL * 365 * (1970 - 1601) - 86400ULL * ((1970 - 1601) / 4) + 86400ULL * ((1970 - 1601) / 100); + info.size = node->size; + free_file (node); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } === modified file 'grub-core/fs/reiserfs.c' --- grub-core/fs/reiserfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/reiserfs.c 2013-03-10 00:05:22 +0000 @@ -875,6 +875,7 @@ entry_v1_stat.rdev, entry_v1_stat.first_direct_byte); #endif + entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v1_stat.size); entry_item->mtime = grub_le_to_cpu32 (entry_v1_stat.mtime); if ((grub_le_to_cpu16 (entry_v1_stat.mode) & S_IFLNK) == S_IFLNK) @@ -923,6 +924,7 @@ entry_v2_stat.blocks, entry_v2_stat.first_direct_byte); #endif + entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v2_stat.size); entry_item->mtime = grub_le_to_cpu32 (entry_v2_stat.mtime); entry_item->size = (grub_off_t) grub_le_to_cpu64 (entry_v2_stat.size); if ((grub_le_to_cpu16 (entry_v2_stat.mode) & S_IFLNK) === modified file 'grub-core/fs/romfs.c' --- grub-core/fs/romfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/romfs.c 2013-03-10 00:16:08 +0000 @@ -331,6 +331,7 @@ grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); + info.size = grub_be_to_cpu32 (node->file.size); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } === modified file 'grub-core/fs/sfs.c' --- grub-core/fs/sfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/sfs.c 2013-03-10 00:16:47 +0000 @@ -671,6 +671,7 @@ info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtime = node->mtime + 8 * 365 * 86400 + 86400 * 2; info.mtimeset = 1; + info.size = node->size; grub_free (node->cache); grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); === modified file 'grub-core/fs/squash4.c' --- grub-core/fs/squash4.c 2013-01-21 01:33:46 +0000 +++ grub-core/fs/squash4.c 2013-03-10 00:17:17 +0000 @@ -656,6 +656,16 @@ info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); info.mtimeset = 1; info.mtime = grub_le_to_cpu32 (node->ino.mtime); + switch (node->ino.type) + { + case grub_cpu_to_le16_compile_time (SQUASH_TYPE_LONG_REGULAR): + info.size = grub_le_to_cpu64 (node->ino.long_file.size); + break; + case grub_cpu_to_le16_compile_time (SQUASH_TYPE_REGULAR): + info.size = grub_le_to_cpu32 (node->ino.file.size); + break; + } + grub_free (node); return ctx->hook (filename, &info, ctx->hook_data); } === modified file 'grub-core/fs/udf.c' --- grub-core/fs/udf.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/udf.c 2013-03-10 00:17:39 +0000 @@ -1026,6 +1026,7 @@ grub_memset (&info, 0, sizeof (info)); info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); + info.size = U64 (node->block.fe.file_size); if (U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE) tstamp = &node->block.fe.modification_time; else if (U16 (node->block.fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_EFE) === modified file 'grub-core/fs/ufs.c' --- grub-core/fs/ufs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/ufs.c 2013-03-10 00:17:58 +0000 @@ -696,6 +696,12 @@ #endif info.mtimeset = 1; +#ifdef MODE_UFS2 + info.size = grub_le_to_cpu64 (inode.size); +#else + info.size = grub_le_to_cpu32 (inode.size); +#endif + if (hook (filename, &info, hook_data)) break; } === modified file 'grub-core/fs/xfs.c' --- grub-core/fs/xfs.c 2013-02-27 16:19:15 +0000 +++ grub-core/fs/xfs.c 2013-03-10 00:18:18 +0000 @@ -731,6 +731,7 @@ { info.mtimeset = 1; info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec); + info.size = grub_be_to_cpu64 (node->inode.size); } info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); grub_free (node); === modified file 'grub-core/fs/zfs/zfs.c' --- grub-core/fs/zfs/zfs.c 2013-01-21 01:33:46 +0000 +++ grub-core/fs/zfs/zfs.c 2013-03-10 00:05:22 +0000 @@ -3768,12 +3768,14 @@ hdrsize = SA_HDR_SIZE (((sa_hdr_phys_t *) sahdrp)); info->mtimeset = 1; info->mtime = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_MTIME_OFFSET), dn.endian); + info->size = grub_zfs_to_cpu64 (grub_get_unaligned64 ((char *) sahdrp + hdrsize + SA_SIZE_OFFSET), dn.endian); } if (dn.dn.dn_bonustype == DMU_OT_ZNODE) { info->mtimeset = 1; info->mtime = grub_zfs_to_cpu64 (((znode_phys_t *) DN_BONUS (&dn.dn))->zp_mtime[0], dn.endian); + info->size = grub_zfs_to_cpu64 (((znode_phys_t *) DN_BONUS (&data->dnode.dn))->zp_size, dn.endian); } return; } === modified file 'include/grub/fs.h' --- include/grub/fs.h 2013-01-21 01:33:46 +0000 +++ include/grub/fs.h 2013-03-10 00:05:22 +0000 @@ -39,6 +39,7 @@ unsigned mtimeset:1; unsigned case_insensitive:1; grub_int32_t mtime; + grub_off_t size; }; typedef int (*grub_fs_dir_hook_t) (const char *filename, === modified file 'util/grub-mount.c' --- util/grub-mount.c 2013-01-21 01:33:46 +0000 +++ util/grub-mount.c 2013-03-10 00:10:49 +0000 @@ -206,14 +206,7 @@ st->st_gid = 0; st->st_rdev = 0; if (!ctx.file_info.dir) - { - grub_file_t file; - file = grub_file_open (path); - if (! file) - return translate_error (); - st->st_size = file->size; - grub_file_close (file); - } + st->st_size = ctx.file_info.size; else st->st_size = 0; st->st_blksize = 512; [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 294 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir 2013-03-10 13:01 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-03-10 13:02 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-11-28 13:30 ` Andrey Borzenkov 1 sibling, 0 replies; 6+ messages in thread From: Andrey Borzenkov @ 2013-11-28 13:30 UTC (permalink / raw) To: grub-devel [-- Attachment #1: Type: text/plain, Size: 2598 bytes --] В Sun, 10 Mar 2013 14:01:53 +0100 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет: > On 09.03.2013 17:47, Andrey Borzenkov wrote: > > > В Fri, 12 Oct 2012 17:09:54 +0100 > > Colin Watson <cjwatson@ubuntu.com> пишет: > > > >> This is very much a temporary hack, so I'm sending it here for > >> discussion rather than just committing it even though it's quite simple. > >> > >> r3036.1.15 introduced support for filling in the attributes of files in > >> fuse_readdir. However, symlinks to directories are passed to call_fill > >> with info.dir unset; call_fill will then try to open them with > >> grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a > >> directory not a regular file. It then causes the whole readdir call to > >> fail. The net effect is that if you, for example, have a symlink > >> anywhere in the top level of a filesystem, then the entire filesystem > >> appears empty in grub-mount. This is the root cause of > >> https://bugs.launchpad.net/bugs/1051306. > >> > >> I think that the proper solution is to pass the full > >> grub_fshelp_filetype to dirhook functions, which would permit > >> implementing true symlink support in grub-mount. That would be a fairly > >> large change that I don't have time for at the moment. As a stopgap, I > >> suggest that we ignore errors from individual grub_file_open calls > >> during fuse_readdir. How does this patch look? > >> > > > > The only reason to call grub_file_open() is to fetch file size, and file > > size is already available when hooks are called. So what about patch > > below? It fixes problem for me, and it trivial enough. This allows > > directory listing to work again. I can extend it with info.is_link to > > return proper filetype to FUSE, but implementing full support needs > > adding readlink that is a separate topic. > > > > I tested it with ext4 and cpio and it works. Testing on more systems > > (in particular, NTFS, which is the only one with non-trivial change) is > > appreciated. > > > > Some time ago I made a similar patch but for another motivation: current > code is way too inefficient for large directories as you have to rescan > directory for every file. The problem with this patch is 10 bytes > increase of core.img. This may be acceptable given this problem (it > happens in ls on runtime as well) and inefficency of scanning. > Any thoughts on it? I'm fine with either approach; there was some reduction of core size in the meantime so may be slight increase is acceptable? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-28 13:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-12 16:09 [PATCH] Ignore symlink traversal failures in grub-mount readdir Colin Watson 2012-10-15 15:08 ` Dave Vasilevsky 2013-03-09 16:47 ` Andrey Borzenkov 2013-03-10 13:01 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-03-10 13:02 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-11-28 13:30 ` Andrey Borzenkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).