| Message ID | 20181026201943.24131-6-stefanrin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | v4 Try to squash metadump data leaks | expand |
On 10/26/18 3:19 PM, Stefan Ring wrote: > --- > db/metadump.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/db/metadump.c b/db/metadump.c > index 39183fb7..bdc6f2f4 100644 > --- a/db/metadump.c > +++ b/db/metadump.c > @@ -2269,6 +2269,24 @@ process_inode_data( > return 1; > } > > +static int > +process_dev_inode( > + xfs_dinode_t *dip) > +{ > + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) || > + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) { > + if (show_warnings) > + print_warning("inode %llu has unexpected extents", > + (unsigned long long)cur_ino); > + return 0; > + } else { > + int used = XFS_DFORK_DPTR(dip) - (char*)dip; > + > + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - used); > + return 1; > + } > +} > + > /* > * when we process the inode, we may change the data in the data and/or > * attribute fork if they are in short form and we are obfuscating names. > @@ -2321,7 +2339,9 @@ process_inode( > case S_IFREG: > success = process_inode_data(dip, TYP_DATA); > break; > - default: ; > + default: > + success = process_dev_inode(dip); > + break; Probably: case S_IFBLK: case S_IFCHR: success = process_dev_inode(dip); break; default: break; no? > } > nametable_clear(); > >
On Mon, Oct 29, 2018 at 4:38 PM Eric Sandeen <sandeen@sandeen.net> wrote: > On 10/26/18 3:19 PM, Stefan Ring wrote: > > --- > > db/metadump.c | 22 +++++++++++++++++++++- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/db/metadump.c b/db/metadump.c > > index 39183fb7..bdc6f2f4 100644 > > --- a/db/metadump.c > > +++ b/db/metadump.c > > @@ -2269,6 +2269,24 @@ process_inode_data( > > return 1; > > } > > > > +static int > > +process_dev_inode( > > + xfs_dinode_t *dip) > > +{ > > + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) || > > + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) { > > + if (show_warnings) > > + print_warning("inode %llu has unexpected extents", > > + (unsigned long long)cur_ino); > > + return 0; > > + } else { > > + int used = XFS_DFORK_DPTR(dip) - (char*)dip; > > + > > + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - used); > > + return 1; > > + } > > +} > > + > > /* > > * when we process the inode, we may change the data in the data and/or > > * attribute fork if they are in short form and we are obfuscating names. > > @@ -2321,7 +2339,9 @@ process_inode( > > case S_IFREG: > > success = process_inode_data(dip, TYP_DATA); > > break; > > - default: ; > > + default: > > + success = process_dev_inode(dip); > > + break; > > Probably: > case S_IFBLK: > case S_IFCHR: > success = process_dev_inode(dip); > break; > default: > break; > > no? Does this catch pipes and sockets, which are also dev inodes? > > > } > > nametable_clear(); > > > >
On 10/29/18 1:33 PM, Stefan Ring wrote: > On Mon, Oct 29, 2018 at 4:38 PM Eric Sandeen <sandeen@sandeen.net> wrote: >> On 10/26/18 3:19 PM, Stefan Ring wrote: >>> --- >>> db/metadump.c | 22 +++++++++++++++++++++- >>> 1 file changed, 21 insertions(+), 1 deletion(-) >>> >>> diff --git a/db/metadump.c b/db/metadump.c >>> index 39183fb7..bdc6f2f4 100644 >>> --- a/db/metadump.c >>> +++ b/db/metadump.c >>> @@ -2269,6 +2269,24 @@ process_inode_data( >>> return 1; >>> } >>> >>> +static int >>> +process_dev_inode( >>> + xfs_dinode_t *dip) >>> +{ >>> + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) || >>> + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) { >>> + if (show_warnings) >>> + print_warning("inode %llu has unexpected extents", >>> + (unsigned long long)cur_ino); >>> + return 0; >>> + } else { >>> + int used = XFS_DFORK_DPTR(dip) - (char*)dip; >>> + >>> + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - used); >>> + return 1; >>> + } >>> +} >>> + >>> /* >>> * when we process the inode, we may change the data in the data and/or >>> * attribute fork if they are in short form and we are obfuscating names. >>> @@ -2321,7 +2339,9 @@ process_inode( >>> case S_IFREG: >>> success = process_inode_data(dip, TYP_DATA); >>> break; >>> - default: ; >>> + default: >>> + success = process_dev_inode(dip); >>> + break; >> >> Probably: >> case S_IFBLK: >> case S_IFCHR: >> success = process_dev_inode(dip); >> break; >> default: >> break; >> >> no? > > Does this catch pipes and sockets, which are also dev inodes? heh, whooops, nope. case S_IFIFO: case S_IFCHR: case S_IFBLK: case S_IFSOCK: ... sorry, my point was just that it's probably better to explicitly name them than to fall through to a default but I forgot about 2 formats. -Eric >> >>> } >>> nametable_clear(); >>> >>> >
On Mon, Oct 29, 2018 at 7:37 PM Eric Sandeen <sandeen@sandeen.net> wrote: > > sorry, my point was just that it's probably better to explicitly name them > than to fall through to a default but I forgot about 2 formats. It's basically down to preference and a choice only you can make. Either you forget one and do too little, or you forget one and do too much. If this is the exhaustive list, I believe you ;).
diff --git a/db/metadump.c b/db/metadump.c index 39183fb7..bdc6f2f4 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -2269,6 +2269,24 @@ process_inode_data( return 1; } +static int +process_dev_inode( + xfs_dinode_t *dip) +{ + if (XFS_DFORK_NEXTENTS(dip, XFS_ATTR_FORK) || + XFS_DFORK_NEXTENTS(dip, XFS_DATA_FORK)) { + if (show_warnings) + print_warning("inode %llu has unexpected extents", + (unsigned long long)cur_ino); + return 0; + } else { + int used = XFS_DFORK_DPTR(dip) - (char*)dip; + + memset(XFS_DFORK_DPTR(dip), 0, mp->m_sb.sb_inodesize - used); + return 1; + } +} + /* * when we process the inode, we may change the data in the data and/or * attribute fork if they are in short form and we are obfuscating names. @@ -2321,7 +2339,9 @@ process_inode( case S_IFREG: success = process_inode_data(dip, TYP_DATA); break; - default: ; + default: + success = process_dev_inode(dip); + break; } nametable_clear();