| Message ID | 20221223025642.33496-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs/154: migrate to python3 | expand |
On Fri, Dec 23, 2022 at 10:56:42AM +0800, Qu Wenruo wrote: > Test case btrfs/154 is still using python2 script, which is already EOL. > Some rolling distros like Archlinux is no longer providing python2 > package anymore. > > This means btrfs/154 will be harder and harder to run. > > To fix the problem, migreate the python script to python3, this involves > the following changes: > > - Change common/config to use python3 > - Strong type conversion between string and bytes > This means, anything involved in the forged bytes has to be bytes. > > And there is no safe way to convert forged bytes into string, unlike > python2. > I guess that's why the author went python2 in the first place. > > Thankfully os.rename() still accepts forged bytes. > > - Use bytes specific checks for invalid chars. > > The updated script can still cause the needed conflicts, can be verified > through "btrfs ins dump-tree" command. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > common/config | 2 +- > src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ > tests/btrfs/154 | 4 ++-- > 3 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/common/config b/common/config > index b2802e5e..e2aba5a9 100644 > --- a/common/config > +++ b/common/config > @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" > export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" > export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" > export THIN_CHECK_PROG="$(type -P thin_check)" > -export PYTHON2_PROG="$(type -P python2)" > +export PYTHON3_PROG="$(type -P python3)" How about: export PYTHON_PROG="$(type -P python)" export PYTHON2_PROG="$(type -P python2)" export PYTHON3_PROG="$(type -P python3)" maybe someone still need python2, or maybe someone doesn't care the version. Thanks, Zorro > export SQLITE3_PROG="$(type -P sqlite3)" > export TIMEOUT_PROG="$(type -P timeout)" > export SETCAP_PROG="$(type -P setcap)" > diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py > index 6c08fcb7..d29bbb70 100755 > --- a/src/btrfs_crc32c_forged_name.py > +++ b/src/btrfs_crc32c_forged_name.py > @@ -59,9 +59,10 @@ class CRC32(object): > # deduce the 4 bytes we need to insert > for c in struct.pack('<L',fwd_crc)[::-1]: > bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] > - bkd_crc ^= ord(c) > + bkd_crc ^= c > > - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] > + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ > + bytes(s[pos:], 'ascii') > return res > > def parse_args(self): > @@ -72,6 +73,12 @@ class CRC32(object): > help="number of forged names to create") > return parser.parse_args() > > +def has_invalid_chars(result: bytes): > + for i in result: > + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): > + return True > + return False > + > if __name__=='__main__': > > crc = CRC32() > @@ -80,12 +87,15 @@ if __name__=='__main__': > args = crc.parse_args() > dirpath=args.dir > while count < args.count : > - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") > + origname = os.urandom (89).hex()[:-1].strip ("\x00") > forgename = crc.forge(wanted_crc, origname, 4) > - if ("/" not in forgename) and ("\x00" not in forgename): > + if not has_invalid_chars(forgename): > srcpath=dirpath + '/' + str(count) > - dstpath=dirpath + '/' + forgename > - file (srcpath, 'a').close() > + # We have to convert all strings to bytes to concatenate the forged > + # name (bytes). > + # Thankfully os.rename() can accept bytes directly. > + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename > + open(srcpath, mode="a").close() > os.rename(srcpath, dstpath) > os.system('btrfs fi sync %s' % (dirpath)) > count+=1; > diff --git a/tests/btrfs/154 b/tests/btrfs/154 > index 240c504c..6be2d5f6 100755 > --- a/tests/btrfs/154 > +++ b/tests/btrfs/154 > @@ -21,7 +21,7 @@ _begin_fstest auto quick > > _supported_fs btrfs > _require_scratch > -_require_command $PYTHON2_PROG python2 > +_require_command $PYTHON3_PROG python3 > > # Currently in btrfs the node/leaf size can not be smaller than the page > # size (but it can be greater than the page size). So use the largest > @@ -42,7 +42,7 @@ _scratch_mount > # ... > # > > -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > echo "Silence is golden" > > # success, all done > -- > 2.39.0 >
On 2022/12/25 19:51, Zorro Lang wrote: > On Fri, Dec 23, 2022 at 10:56:42AM +0800, Qu Wenruo wrote: >> Test case btrfs/154 is still using python2 script, which is already EOL. >> Some rolling distros like Archlinux is no longer providing python2 >> package anymore. >> >> This means btrfs/154 will be harder and harder to run. >> >> To fix the problem, migreate the python script to python3, this involves >> the following changes: >> >> - Change common/config to use python3 >> - Strong type conversion between string and bytes >> This means, anything involved in the forged bytes has to be bytes. >> >> And there is no safe way to convert forged bytes into string, unlike >> python2. >> I guess that's why the author went python2 in the first place. >> >> Thankfully os.rename() still accepts forged bytes. >> >> - Use bytes specific checks for invalid chars. >> >> The updated script can still cause the needed conflicts, can be verified >> through "btrfs ins dump-tree" command. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> common/config | 2 +- >> src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ >> tests/btrfs/154 | 4 ++-- >> 3 files changed, 19 insertions(+), 9 deletions(-) >> >> diff --git a/common/config b/common/config >> index b2802e5e..e2aba5a9 100644 >> --- a/common/config >> +++ b/common/config >> @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" >> export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" >> export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" >> export THIN_CHECK_PROG="$(type -P thin_check)" >> -export PYTHON2_PROG="$(type -P python2)" >> +export PYTHON3_PROG="$(type -P python3)" > > How about: > > export PYTHON_PROG="$(type -P python)" > export PYTHON2_PROG="$(type -P python2)" > export PYTHON3_PROG="$(type -P python3)" > > maybe someone still need python2, or maybe someone doesn't care the version. Even for distros which completely get rid of python2 and make python3 default, there is still python3. And python is just a softlink to python3, which further links to the real python $ type -P python2 $ type -P python3 /usr/bin/python3 $ type -P python /usr/bin/python $ ls -alh /usr/bin/python lrwxrwxrwx 1 root root 7 Nov 1 22:18 /usr/bin/python -> python3 $ ls -alh /usr/bin/python3 lrwxrwxrwx 1 root root 10 Nov 1 22:18 /usr/bin/python3 -> python3.10 Secondly, for this particular case, we must distinguish python2 and python3, especially due to the strong requirement for string encoding. So to your PYTHON_PROG usage, no, it's not good at all, it's going to be distro dependent and will be a disaster if some one is using PYTHON_PROG. For PYTHON2_PROG, there is no usage of it any more, thus I see now reason to define it. Thanks, Qu > > Thanks, > Zorro > >> export SQLITE3_PROG="$(type -P sqlite3)" >> export TIMEOUT_PROG="$(type -P timeout)" >> export SETCAP_PROG="$(type -P setcap)" >> diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py >> index 6c08fcb7..d29bbb70 100755 >> --- a/src/btrfs_crc32c_forged_name.py >> +++ b/src/btrfs_crc32c_forged_name.py >> @@ -59,9 +59,10 @@ class CRC32(object): >> # deduce the 4 bytes we need to insert >> for c in struct.pack('<L',fwd_crc)[::-1]: >> bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] >> - bkd_crc ^= ord(c) >> + bkd_crc ^= c >> >> - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] >> + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ >> + bytes(s[pos:], 'ascii') >> return res >> >> def parse_args(self): >> @@ -72,6 +73,12 @@ class CRC32(object): >> help="number of forged names to create") >> return parser.parse_args() >> >> +def has_invalid_chars(result: bytes): >> + for i in result: >> + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): >> + return True >> + return False >> + >> if __name__=='__main__': >> >> crc = CRC32() >> @@ -80,12 +87,15 @@ if __name__=='__main__': >> args = crc.parse_args() >> dirpath=args.dir >> while count < args.count : >> - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") >> + origname = os.urandom (89).hex()[:-1].strip ("\x00") >> forgename = crc.forge(wanted_crc, origname, 4) >> - if ("/" not in forgename) and ("\x00" not in forgename): >> + if not has_invalid_chars(forgename): >> srcpath=dirpath + '/' + str(count) >> - dstpath=dirpath + '/' + forgename >> - file (srcpath, 'a').close() >> + # We have to convert all strings to bytes to concatenate the forged >> + # name (bytes). >> + # Thankfully os.rename() can accept bytes directly. >> + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename >> + open(srcpath, mode="a").close() >> os.rename(srcpath, dstpath) >> os.system('btrfs fi sync %s' % (dirpath)) >> count+=1; >> diff --git a/tests/btrfs/154 b/tests/btrfs/154 >> index 240c504c..6be2d5f6 100755 >> --- a/tests/btrfs/154 >> +++ b/tests/btrfs/154 >> @@ -21,7 +21,7 @@ _begin_fstest auto quick >> >> _supported_fs btrfs >> _require_scratch >> -_require_command $PYTHON2_PROG python2 >> +_require_command $PYTHON3_PROG python3 >> >> # Currently in btrfs the node/leaf size can not be smaller than the page >> # size (but it can be greater than the page size). So use the largest >> @@ -42,7 +42,7 @@ _scratch_mount >> # ... >> # >> >> -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 >> +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 >> echo "Silence is golden" >> >> # success, all done >> -- >> 2.39.0 >> >
On Sun, Dec 25, 2022 at 08:11:02PM +0800, Qu Wenruo wrote: > > > On 2022/12/25 19:51, Zorro Lang wrote: > > On Fri, Dec 23, 2022 at 10:56:42AM +0800, Qu Wenruo wrote: > > > Test case btrfs/154 is still using python2 script, which is already EOL. > > > Some rolling distros like Archlinux is no longer providing python2 > > > package anymore. > > > > > > This means btrfs/154 will be harder and harder to run. > > > > > > To fix the problem, migreate the python script to python3, this involves > > > the following changes: > > > > > > - Change common/config to use python3 > > > - Strong type conversion between string and bytes > > > This means, anything involved in the forged bytes has to be bytes. > > > > > > And there is no safe way to convert forged bytes into string, unlike > > > python2. > > > I guess that's why the author went python2 in the first place. > > > > > > Thankfully os.rename() still accepts forged bytes. > > > > > > - Use bytes specific checks for invalid chars. > > > > > > The updated script can still cause the needed conflicts, can be verified > > > through "btrfs ins dump-tree" command. > > > > > > Signed-off-by: Qu Wenruo <wqu@suse.com> > > > --- > > > common/config | 2 +- > > > src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ > > > tests/btrfs/154 | 4 ++-- > > > 3 files changed, 19 insertions(+), 9 deletions(-) > > > > > > diff --git a/common/config b/common/config > > > index b2802e5e..e2aba5a9 100644 > > > --- a/common/config > > > +++ b/common/config > > > @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" > > > export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" > > > export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" > > > export THIN_CHECK_PROG="$(type -P thin_check)" > > > -export PYTHON2_PROG="$(type -P python2)" > > > +export PYTHON3_PROG="$(type -P python3)" > > > > How about: > > > > export PYTHON_PROG="$(type -P python)" > > export PYTHON2_PROG="$(type -P python2)" > > export PYTHON3_PROG="$(type -P python3)" > > > > maybe someone still need python2, or maybe someone doesn't care the version. > > Even for distros which completely get rid of python2 and make python3 > default, there is still python3. > And python is just a softlink to python3, which further links to the real > python > > $ type -P python2 > $ type -P python3 > /usr/bin/python3 > $ type -P python > /usr/bin/python > > $ ls -alh /usr/bin/python > lrwxrwxrwx 1 root root 7 Nov 1 22:18 /usr/bin/python -> python3 > $ ls -alh /usr/bin/python3 > lrwxrwxrwx 1 root root 10 Nov 1 22:18 /usr/bin/python3 -> python3.10 > > Secondly, for this particular case, we must distinguish python2 and python3, > especially due to the strong requirement for string encoding. > > > So to your PYTHON_PROG usage, no, it's not good at all, it's going to be > distro dependent and will be a disaster if some one is using PYTHON_PROG. > > For PYTHON2_PROG, there is no usage of it any more, thus I see now reason to > define it. OK, due to only btrfs/154 uses this PYTHON?_PROG thing now, we can talk about that later. This patch looks good to me. Reviewed-by: Zorro Lang <zlang@redhat.com> > > Thanks, > Qu > > > > Thanks, > > Zorro > > > > > export SQLITE3_PROG="$(type -P sqlite3)" > > > export TIMEOUT_PROG="$(type -P timeout)" > > > export SETCAP_PROG="$(type -P setcap)" > > > diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py > > > index 6c08fcb7..d29bbb70 100755 > > > --- a/src/btrfs_crc32c_forged_name.py > > > +++ b/src/btrfs_crc32c_forged_name.py > > > @@ -59,9 +59,10 @@ class CRC32(object): > > > # deduce the 4 bytes we need to insert > > > for c in struct.pack('<L',fwd_crc)[::-1]: > > > bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] > > > - bkd_crc ^= ord(c) > > > + bkd_crc ^= c > > > - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] > > > + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ > > > + bytes(s[pos:], 'ascii') > > > return res > > > def parse_args(self): > > > @@ -72,6 +73,12 @@ class CRC32(object): > > > help="number of forged names to create") > > > return parser.parse_args() > > > +def has_invalid_chars(result: bytes): > > > + for i in result: > > > + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): > > > + return True > > > + return False > > > + > > > if __name__=='__main__': > > > crc = CRC32() > > > @@ -80,12 +87,15 @@ if __name__=='__main__': > > > args = crc.parse_args() > > > dirpath=args.dir > > > while count < args.count : > > > - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") > > > + origname = os.urandom (89).hex()[:-1].strip ("\x00") > > > forgename = crc.forge(wanted_crc, origname, 4) > > > - if ("/" not in forgename) and ("\x00" not in forgename): > > > + if not has_invalid_chars(forgename): > > > srcpath=dirpath + '/' + str(count) > > > - dstpath=dirpath + '/' + forgename > > > - file (srcpath, 'a').close() > > > + # We have to convert all strings to bytes to concatenate the forged > > > + # name (bytes). > > > + # Thankfully os.rename() can accept bytes directly. > > > + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename > > > + open(srcpath, mode="a").close() > > > os.rename(srcpath, dstpath) > > > os.system('btrfs fi sync %s' % (dirpath)) > > > count+=1; > > > diff --git a/tests/btrfs/154 b/tests/btrfs/154 > > > index 240c504c..6be2d5f6 100755 > > > --- a/tests/btrfs/154 > > > +++ b/tests/btrfs/154 > > > @@ -21,7 +21,7 @@ _begin_fstest auto quick > > > _supported_fs btrfs > > > _require_scratch > > > -_require_command $PYTHON2_PROG python2 > > > +_require_command $PYTHON3_PROG python3 > > > # Currently in btrfs the node/leaf size can not be smaller than the page > > > # size (but it can be greater than the page size). So use the largest > > > @@ -42,7 +42,7 @@ _scratch_mount > > > # ... > > > # > > > -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > > > +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > > > echo "Silence is golden" > > > # success, all done > > > -- > > > 2.39.0 > > > > > >
On 2022/12/25 20:50, Zorro Lang wrote: > On Sun, Dec 25, 2022 at 08:11:02PM +0800, Qu Wenruo wrote: >> >> >> On 2022/12/25 19:51, Zorro Lang wrote: >>> On Fri, Dec 23, 2022 at 10:56:42AM +0800, Qu Wenruo wrote: >>>> Test case btrfs/154 is still using python2 script, which is already EOL. >>>> Some rolling distros like Archlinux is no longer providing python2 >>>> package anymore. >>>> >>>> This means btrfs/154 will be harder and harder to run. >>>> >>>> To fix the problem, migreate the python script to python3, this involves >>>> the following changes: >>>> >>>> - Change common/config to use python3 >>>> - Strong type conversion between string and bytes >>>> This means, anything involved in the forged bytes has to be bytes. >>>> >>>> And there is no safe way to convert forged bytes into string, unlike >>>> python2. >>>> I guess that's why the author went python2 in the first place. >>>> >>>> Thankfully os.rename() still accepts forged bytes. >>>> >>>> - Use bytes specific checks for invalid chars. >>>> >>>> The updated script can still cause the needed conflicts, can be verified >>>> through "btrfs ins dump-tree" command. >>>> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> common/config | 2 +- >>>> src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ >>>> tests/btrfs/154 | 4 ++-- >>>> 3 files changed, 19 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/common/config b/common/config >>>> index b2802e5e..e2aba5a9 100644 >>>> --- a/common/config >>>> +++ b/common/config >>>> @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" >>>> export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" >>>> export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" >>>> export THIN_CHECK_PROG="$(type -P thin_check)" >>>> -export PYTHON2_PROG="$(type -P python2)" >>>> +export PYTHON3_PROG="$(type -P python3)" >>> >>> How about: >>> >>> export PYTHON_PROG="$(type -P python)" >>> export PYTHON2_PROG="$(type -P python2)" >>> export PYTHON3_PROG="$(type -P python3)" >>> >>> maybe someone still need python2, or maybe someone doesn't care the version. >> >> Even for distros which completely get rid of python2 and make python3 >> default, there is still python3. >> And python is just a softlink to python3, which further links to the real >> python >> >> $ type -P python2 >> $ type -P python3 >> /usr/bin/python3 >> $ type -P python >> /usr/bin/python >> >> $ ls -alh /usr/bin/python >> lrwxrwxrwx 1 root root 7 Nov 1 22:18 /usr/bin/python -> python3 >> $ ls -alh /usr/bin/python3 >> lrwxrwxrwx 1 root root 10 Nov 1 22:18 /usr/bin/python3 -> python3.10 >> >> Secondly, for this particular case, we must distinguish python2 and python3, >> especially due to the strong requirement for string encoding. >> >> >> So to your PYTHON_PROG usage, no, it's not good at all, it's going to be >> distro dependent and will be a disaster if some one is using PYTHON_PROG. >> >> For PYTHON2_PROG, there is no usage of it any more, thus I see now reason to >> define it. > > OK, due to only btrfs/154 uses this PYTHON?_PROG thing now, we can talk about > that later. This patch looks good to me. In fact, I'd prefer to use C to do the CRC32C hash conflicts, especially considering how hard it is already in python3 to parse any invalid bytes. (And some other weird behaviors, like using hard coded filename name lengh 89, which later converts to hex string and has to get rid of invalid chars like \0 and \/) But that would be another (small) project to go. Until then, let's keep the single (and bad example) python3. Thanks, Qu > > Reviewed-by: Zorro Lang <zlang@redhat.com> > >> >> Thanks, >> Qu >>> >>> Thanks, >>> Zorro >>> >>>> export SQLITE3_PROG="$(type -P sqlite3)" >>>> export TIMEOUT_PROG="$(type -P timeout)" >>>> export SETCAP_PROG="$(type -P setcap)" >>>> diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py >>>> index 6c08fcb7..d29bbb70 100755 >>>> --- a/src/btrfs_crc32c_forged_name.py >>>> +++ b/src/btrfs_crc32c_forged_name.py >>>> @@ -59,9 +59,10 @@ class CRC32(object): >>>> # deduce the 4 bytes we need to insert >>>> for c in struct.pack('<L',fwd_crc)[::-1]: >>>> bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] >>>> - bkd_crc ^= ord(c) >>>> + bkd_crc ^= c >>>> - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] >>>> + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ >>>> + bytes(s[pos:], 'ascii') >>>> return res >>>> def parse_args(self): >>>> @@ -72,6 +73,12 @@ class CRC32(object): >>>> help="number of forged names to create") >>>> return parser.parse_args() >>>> +def has_invalid_chars(result: bytes): >>>> + for i in result: >>>> + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): >>>> + return True >>>> + return False >>>> + >>>> if __name__=='__main__': >>>> crc = CRC32() >>>> @@ -80,12 +87,15 @@ if __name__=='__main__': >>>> args = crc.parse_args() >>>> dirpath=args.dir >>>> while count < args.count : >>>> - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") >>>> + origname = os.urandom (89).hex()[:-1].strip ("\x00") >>>> forgename = crc.forge(wanted_crc, origname, 4) >>>> - if ("/" not in forgename) and ("\x00" not in forgename): >>>> + if not has_invalid_chars(forgename): >>>> srcpath=dirpath + '/' + str(count) >>>> - dstpath=dirpath + '/' + forgename >>>> - file (srcpath, 'a').close() >>>> + # We have to convert all strings to bytes to concatenate the forged >>>> + # name (bytes). >>>> + # Thankfully os.rename() can accept bytes directly. >>>> + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename >>>> + open(srcpath, mode="a").close() >>>> os.rename(srcpath, dstpath) >>>> os.system('btrfs fi sync %s' % (dirpath)) >>>> count+=1; >>>> diff --git a/tests/btrfs/154 b/tests/btrfs/154 >>>> index 240c504c..6be2d5f6 100755 >>>> --- a/tests/btrfs/154 >>>> +++ b/tests/btrfs/154 >>>> @@ -21,7 +21,7 @@ _begin_fstest auto quick >>>> _supported_fs btrfs >>>> _require_scratch >>>> -_require_command $PYTHON2_PROG python2 >>>> +_require_command $PYTHON3_PROG python3 >>>> # Currently in btrfs the node/leaf size can not be smaller than the page >>>> # size (but it can be greater than the page size). So use the largest >>>> @@ -42,7 +42,7 @@ _scratch_mount >>>> # ... >>>> # >>>> -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 >>>> +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 >>>> echo "Silence is golden" >>>> # success, all done >>>> -- >>>> 2.39.0 >>>> >>> >> >
On Thu, Dec 22, 2022 at 10:03 PM Qu Wenruo <wqu@suse.com> wrote: > > Test case btrfs/154 is still using python2 script, which is already EOL. > Some rolling distros like Archlinux is no longer providing python2 > package anymore. > > This means btrfs/154 will be harder and harder to run. > > To fix the problem, migreate the python script to python3, this involves > the following changes: > > - Change common/config to use python3 > - Strong type conversion between string and bytes > This means, anything involved in the forged bytes has to be bytes. > > And there is no safe way to convert forged bytes into string, unlike > python2. > I guess that's why the author went python2 in the first place. > > Thankfully os.rename() still accepts forged bytes. > > - Use bytes specific checks for invalid chars. > > The updated script can still cause the needed conflicts, can be verified > through "btrfs ins dump-tree" command. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > common/config | 2 +- > src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ > tests/btrfs/154 | 4 ++-- > 3 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/common/config b/common/config > index b2802e5e..e2aba5a9 100644 > --- a/common/config > +++ b/common/config > @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" > export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" > export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" > export THIN_CHECK_PROG="$(type -P thin_check)" > -export PYTHON2_PROG="$(type -P python2)" > +export PYTHON3_PROG="$(type -P python3)" > export SQLITE3_PROG="$(type -P sqlite3)" > export TIMEOUT_PROG="$(type -P timeout)" > export SETCAP_PROG="$(type -P setcap)" > diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py > index 6c08fcb7..d29bbb70 100755 > --- a/src/btrfs_crc32c_forged_name.py > +++ b/src/btrfs_crc32c_forged_name.py > @@ -59,9 +59,10 @@ class CRC32(object): > # deduce the 4 bytes we need to insert > for c in struct.pack('<L',fwd_crc)[::-1]: > bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] > - bkd_crc ^= ord(c) > + bkd_crc ^= c > > - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] > + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ > + bytes(s[pos:], 'ascii') > return res > > def parse_args(self): > @@ -72,6 +73,12 @@ class CRC32(object): > help="number of forged names to create") > return parser.parse_args() > > +def has_invalid_chars(result: bytes): > + for i in result: > + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): > + return True > + return False > + > if __name__=='__main__': > > crc = CRC32() > @@ -80,12 +87,15 @@ if __name__=='__main__': > args = crc.parse_args() > dirpath=args.dir > while count < args.count : > - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") > + origname = os.urandom (89).hex()[:-1].strip ("\x00") > forgename = crc.forge(wanted_crc, origname, 4) > - if ("/" not in forgename) and ("\x00" not in forgename): > + if not has_invalid_chars(forgename): > srcpath=dirpath + '/' + str(count) > - dstpath=dirpath + '/' + forgename > - file (srcpath, 'a').close() > + # We have to convert all strings to bytes to concatenate the forged > + # name (bytes). > + # Thankfully os.rename() can accept bytes directly. > + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename > + open(srcpath, mode="a").close() > os.rename(srcpath, dstpath) > os.system('btrfs fi sync %s' % (dirpath)) > count+=1; > diff --git a/tests/btrfs/154 b/tests/btrfs/154 > index 240c504c..6be2d5f6 100755 > --- a/tests/btrfs/154 > +++ b/tests/btrfs/154 > @@ -21,7 +21,7 @@ _begin_fstest auto quick > > _supported_fs btrfs > _require_scratch > -_require_command $PYTHON2_PROG python2 > +_require_command $PYTHON3_PROG python3 > > # Currently in btrfs the node/leaf size can not be smaller than the page > # size (but it can be greater than the page size). So use the largest > @@ -42,7 +42,7 @@ _scratch_mount > # ... > # > > -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 > echo "Silence is golden" > > # success, all done > -- > 2.39.0 > This test makes my eyes bleed, but that's not a reason to say no to this patch... Reviewed-by: Neal Gompa <neal@gompa.dev>
diff --git a/common/config b/common/config index b2802e5e..e2aba5a9 100644 --- a/common/config +++ b/common/config @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)" export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)" export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)" export THIN_CHECK_PROG="$(type -P thin_check)" -export PYTHON2_PROG="$(type -P python2)" +export PYTHON3_PROG="$(type -P python3)" export SQLITE3_PROG="$(type -P sqlite3)" export TIMEOUT_PROG="$(type -P timeout)" export SETCAP_PROG="$(type -P setcap)" diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py index 6c08fcb7..d29bbb70 100755 --- a/src/btrfs_crc32c_forged_name.py +++ b/src/btrfs_crc32c_forged_name.py @@ -59,9 +59,10 @@ class CRC32(object): # deduce the 4 bytes we need to insert for c in struct.pack('<L',fwd_crc)[::-1]: bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24] - bkd_crc ^= ord(c) + bkd_crc ^= c - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:] + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \ + bytes(s[pos:], 'ascii') return res def parse_args(self): @@ -72,6 +73,12 @@ class CRC32(object): help="number of forged names to create") return parser.parse_args() +def has_invalid_chars(result: bytes): + for i in result: + if i == 0 or i == int.from_bytes(b'/', byteorder="little"): + return True + return False + if __name__=='__main__': crc = CRC32() @@ -80,12 +87,15 @@ if __name__=='__main__': args = crc.parse_args() dirpath=args.dir while count < args.count : - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00") + origname = os.urandom (89).hex()[:-1].strip ("\x00") forgename = crc.forge(wanted_crc, origname, 4) - if ("/" not in forgename) and ("\x00" not in forgename): + if not has_invalid_chars(forgename): srcpath=dirpath + '/' + str(count) - dstpath=dirpath + '/' + forgename - file (srcpath, 'a').close() + # We have to convert all strings to bytes to concatenate the forged + # name (bytes). + # Thankfully os.rename() can accept bytes directly. + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename + open(srcpath, mode="a").close() os.rename(srcpath, dstpath) os.system('btrfs fi sync %s' % (dirpath)) count+=1; diff --git a/tests/btrfs/154 b/tests/btrfs/154 index 240c504c..6be2d5f6 100755 --- a/tests/btrfs/154 +++ b/tests/btrfs/154 @@ -21,7 +21,7 @@ _begin_fstest auto quick _supported_fs btrfs _require_scratch -_require_command $PYTHON2_PROG python2 +_require_command $PYTHON3_PROG python3 # Currently in btrfs the node/leaf size can not be smaller than the page # size (but it can be greater than the page size). So use the largest @@ -42,7 +42,7 @@ _scratch_mount # ... # -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310 echo "Silence is golden" # success, all done
Test case btrfs/154 is still using python2 script, which is already EOL. Some rolling distros like Archlinux is no longer providing python2 package anymore. This means btrfs/154 will be harder and harder to run. To fix the problem, migreate the python script to python3, this involves the following changes: - Change common/config to use python3 - Strong type conversion between string and bytes This means, anything involved in the forged bytes has to be bytes. And there is no safe way to convert forged bytes into string, unlike python2. I guess that's why the author went python2 in the first place. Thankfully os.rename() still accepts forged bytes. - Use bytes specific checks for invalid chars. The updated script can still cause the needed conflicts, can be verified through "btrfs ins dump-tree" command. Signed-off-by: Qu Wenruo <wqu@suse.com> --- common/config | 2 +- src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------ tests/btrfs/154 | 4 ++-- 3 files changed, 19 insertions(+), 9 deletions(-)