[Qemu-devel] [PATCH 1/2] vdi: Fix error message and two more small code improvements

[Qemu-devel] [PATCH 1/2] vdi: Fix error message and two more small code improvements

[Stefan Weil]

* Fix wrong error message (copy+paste bug from commit
  5b7aa9b56d1bfc79916262f380c3fc7961becb50).

* Replace the default cluster size of 1 * MiB by DEFAULT_CLUSTER_SIZE.

* Don't check for the default cluster size if we support other sizes, too.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vdi.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index ac9a025..b832905 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -420,11 +420,13 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    header.sector_size, SECTOR_SIZE);
         ret = -ENOTSUP;
         goto fail;
-    } else if (header.block_size != 1 * MiB) {
-        error_setg(errp, "unsupported VDI image (sector size %u is not %u)",
-                   header.block_size, 1 * MiB);
+#if !defined(CONFIG_VDI_BLOCK_SIZE)
+    } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
+        error_setg(errp, "unsupported VDI image (block size %u is not %u)",
+                   header.block_size, DEFAULT_CLUSTER_SIZE);
         ret = -ENOTSUP;
         goto fail;
+#endif
     } else if (header.disk_size >
                (uint64_t)header.blocks_in_image * header.block_size) {
         error_setg(errp, "unsupported VDI image (disk size %" PRIu64 ", "
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/2] vdi: add bounds checks for block related header fields (CVE-2014-0144)

[Stefan Weil]

(1) block_size must not be null.

(2) blocks_in_image * 4 must fit into a size_t.

(3) blocks_in_image * block_size must fit into a uint64_t.

Header field disk_size already has a bounds check which now works
because of modification (1) and (3).

This patch was inspired by Jeff Cody's patch for the same problem.

Cc: Jeff Cody <jcody@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

Hello Stefan, hello Jeff,

I tried to improve your previous patch - maybe you want to improve it further
or take parts from it to fix that CVE (of which I have no information).

The patch was compiled on 32 and 64 bit Linux and cross compiled with MinGW-w64,
but not tested otherwise.

Regards
Stefan

 block/vdi.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index b832905..8fb59a1 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -420,7 +420,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    header.sector_size, SECTOR_SIZE);
         ret = -ENOTSUP;
         goto fail;
-#if !defined(CONFIG_VDI_BLOCK_SIZE)
+#if defined(CONFIG_VDI_BLOCK_SIZE)
+    } else if (header.block_size == 0) {
+        error_setg(errp, "unsupported VDI image (block size is null)");
+        ret = -EINVAL;
+        goto fail;
+#else
     } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
         error_setg(errp, "unsupported VDI image (block size %u is not %u)",
                    header.block_size, DEFAULT_CLUSTER_SIZE);
@@ -435,6 +440,18 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    (uint64_t)header.blocks_in_image * header.block_size);
         ret = -ENOTSUP;
         goto fail;
+#if SIZE_MAX <= UINT32_MAX
+    } else if (header.blocks_in_image > SIZE_MAX / sizeof(uint32_t)) {
+        /* blocks_in_image * sizeof(uint32_t) must fit into size_t. */
+        error_setg(errp, "unsupported VDI image (number of blocks %u, "
+                   "only %zu are possible)",
+                   header.blocks_in_image, SIZE_MAX / sizeof(uint32_t));
+#endif
+    } else if (header.blocks_in_image > UINT64_MAX / header.block_size) {
+        /* blocks_in_image * block_size must fit into uint64_t. */
+        error_setg(errp, "unsupported VDI image (number of blocks %u, "
+                   "only %" PRIu64 " are possible)",
+                   header.blocks_in_image, UINT64_MAX / header.block_size);
     } else if (!uuid_is_null(header.uuid_link)) {
         error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
         ret = -ENOTSUP;
@@ -691,6 +708,33 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
+#if defined(CONFIG_VDI_BLOCK_SIZE)
+    if (block_size == 0) {
+        return -EINVAL;
+    }
+#endif
+
+    if (bytes > UINT64_MAX - block_size) {
+        /* Overflow in calculation of blocks (see below). */
+        return -ENOTSUP;
+    }
+
+    /* We need enough blocks to store the given disk size,
+       so always round up. */
+    blocks = (bytes + block_size - 1) / block_size;
+
+#if SIZE_MAX <= UINT32_MAX
+    if (blocks > SIZE_MAX / sizeof(uint32_t)) {
+        /* blocks * sizeof(uint32_t) must fit into size_t. */
+        return -ENOTSUP;
+    }
+#endif
+
+    if (blocks > UINT64_MAX / block_size) {
+        /* blocks * block_size must fit into uint64_t. */
+        return -EINVAL;
+    }
+
     fd = qemu_open(filename,
                    O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
                    0644);
@@ -698,10 +742,6 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
         return -errno;
     }
 
-    /* We need enough blocks to store the given disk size,
-       so always round up. */
-    blocks = (bytes + block_size - 1) / block_size;
-
     bmap_size = blocks * sizeof(uint32_t);
     bmap_size = ((bmap_size + SECTOR_SIZE - 1) & ~(SECTOR_SIZE -1));
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 2/2] vdi: add bounds checks for block related header fields (CVE-2014-0144)

[Jeff Cody]

On Wed, Mar 26, 2014 at 10:38:05PM +0100, Stefan Weil wrote:
> (1) block_size must not be null.
> 
> (2) blocks_in_image * 4 must fit into a size_t.
> 
> (3) blocks_in_image * block_size must fit into a uint64_t.
> 
> Header field disk_size already has a bounds check which now works
> because of modification (1) and (3).
> 
> This patch was inspired by Jeff Cody's patch for the same problem.
> 
> Cc: Jeff Cody <jcody@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> Hello Stefan, hello Jeff,
> 
> I tried to improve your previous patch - maybe you want to improve it further
> or take parts from it to fix that CVE (of which I have no information).
>

Thanks!

> The patch was compiled on 32 and 64 bit Linux and cross compiled with MinGW-w64,
> but not tested otherwise.
> 
> Regards
> Stefan
> 
>  block/vdi.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index b832905..8fb59a1 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -420,7 +420,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>                     header.sector_size, SECTOR_SIZE);
>          ret = -ENOTSUP;
>          goto fail;
> -#if !defined(CONFIG_VDI_BLOCK_SIZE)

Is this from qemu/master?  I don't see the above in the master
branch.

> +#if defined(CONFIG_VDI_BLOCK_SIZE)
> +    } else if (header.block_size == 0) {
> +        error_setg(errp, "unsupported VDI image (block size is null)");
> +        ret = -EINVAL;
> +        goto fail;
> +#else

The above is essentially dead code, since CONFIG_VDI_BLOCK_SIZE is
commented out.  Should this be added to configure, so that it can be
compiled in like other options?  I think if it is worth putting the
code in the driver, it probably makes sense to have a configure option
for it.  Otherwise, in my opinion, it is generally best to excise dead
code.

With regards to adding this prior to QEMU 2.0, this would be more in
line with a feature addition, I think (i.e., the support of
non-default block sizes).

>      } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
>          error_setg(errp, "unsupported VDI image (block size %u is not %u)",
>                     header.block_size, DEFAULT_CLUSTER_SIZE);
> @@ -435,6 +440,18 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>                     (uint64_t)header.blocks_in_image * header.block_size);
>          ret = -ENOTSUP;
>          goto fail;
> +#if SIZE_MAX <= UINT32_MAX
> +    } else if (header.blocks_in_image > SIZE_MAX / sizeof(uint32_t)) {
> +        /* blocks_in_image * sizeof(uint32_t) must fit into size_t. */
> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
> +                   "only %zu are possible)",
> +                   header.blocks_in_image, SIZE_MAX / sizeof(uint32_t));
> +#endif
> +    } else if (header.blocks_in_image > UINT64_MAX / header.block_size) {
> +        /* blocks_in_image * block_size must fit into uint64_t. */
> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
> +                   "only %" PRIu64 " are possible)",
> +                   header.blocks_in_image, UINT64_MAX / header.block_size);

Since header.blocks_in_image is a uint32_t, this means the driver may
allocate up to 16GB of ram... that is my main concern here.


>      } else if (!uuid_is_null(header.uuid_link)) {
>          error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
>          ret = -ENOTSUP;
> @@ -691,6 +708,33 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
>          options++;
>      }
>  
> +#if defined(CONFIG_VDI_BLOCK_SIZE)
> +    if (block_size == 0) {
> +        return -EINVAL;
> +    }
> +#endif
> +

Same concern as above with regards to dead code / new feature


> +    if (bytes > UINT64_MAX - block_size) {
> +        /* Overflow in calculation of blocks (see below). */
> +        return -ENOTSUP;
> +    }
> +
> +    /* We need enough blocks to store the given disk size,
> +       so always round up. */
> +    blocks = (bytes + block_size - 1) / block_size;
> +
> +#if SIZE_MAX <= UINT32_MAX
> +    if (blocks > SIZE_MAX / sizeof(uint32_t)) {
> +        /* blocks * sizeof(uint32_t) must fit into size_t. */
> +        return -ENOTSUP;
> +    }
> +#endif
> +
> +    if (blocks > UINT64_MAX / block_size) {
> +        /* blocks * block_size must fit into uint64_t. */
> +        return -EINVAL;
> +    }
> +
>      fd = qemu_open(filename,
>                     O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
>                     0644);
> @@ -698,10 +742,6 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
>          return -errno;
>      }
>  
> -    /* We need enough blocks to store the given disk size,
> -       so always round up. */
> -    blocks = (bytes + block_size - 1) / block_size;
> -
>      bmap_size = blocks * sizeof(uint32_t);
>      bmap_size = ((bmap_size + SECTOR_SIZE - 1) & ~(SECTOR_SIZE -1));
>  
> -- 
> 1.7.10.4
> 

Re: [Qemu-devel] [PATCH 2/2] vdi: add bounds checks for block related header fields (CVE-2014-0144)

[Stefan Weil]

Am 27.03.2014 20:49, schrieb Jeff Cody:
> On Wed, Mar 26, 2014 at 10:38:05PM +0100, Stefan Weil wrote:
>> (1) block_size must not be null.
>>
>> (2) blocks_in_image * 4 must fit into a size_t.
>>
>> (3) blocks_in_image * block_size must fit into a uint64_t.
>>
>> Header field disk_size already has a bounds check which now works
>> because of modification (1) and (3).
>>
>> This patch was inspired by Jeff Cody's patch for the same problem.
>>
>> Cc: Jeff Cody <jcody@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> Hello Stefan, hello Jeff,
>>
>> I tried to improve your previous patch - maybe you want to improve it further
>> or take parts from it to fix that CVE (of which I have no information).
>>
> 
> Thanks!
> 
>> The patch was compiled on 32 and 64 bit Linux and cross compiled with MinGW-w64,
>> but not tested otherwise.
>>
>> Regards
>> Stefan
>>
>>  block/vdi.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 45 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/vdi.c b/block/vdi.c
>> index b832905..8fb59a1 100644
>> --- a/block/vdi.c
>> +++ b/block/vdi.c
>> @@ -420,7 +420,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>>                     header.sector_size, SECTOR_SIZE);
>>          ret = -ENOTSUP;
>>          goto fail;
>> -#if !defined(CONFIG_VDI_BLOCK_SIZE)
> 
> Is this from qemu/master?  I don't see the above in the master
> branch.

It's from the previous patch (PATCH 1/2):
http://patchwork.ozlabs.org/patch/334116/. You have to apply both
patches in sequence.

> 
>> +#if defined(CONFIG_VDI_BLOCK_SIZE)
>> +    } else if (header.block_size == 0) {
>> +        error_setg(errp, "unsupported VDI image (block size is null)");
>> +        ret = -EINVAL;
>> +        goto fail;
>> +#else
> 
> The above is essentially dead code, since CONFIG_VDI_BLOCK_SIZE is
> commented out.  Should this be added to configure, so that it can be
> compiled in like other options?  I think if it is worth putting the
> code in the driver, it probably makes sense to have a configure option
> for it.  Otherwise, in my opinion, it is generally best to excise dead
> code.
> 
> With regards to adding this prior to QEMU 2.0, this would be more in
> line with a feature addition, I think (i.e., the support of
> non-default block sizes).
> 

Yes, it would be a very bad idea to activate variable block sizes for
QEMU 2.0. I don't think adding a configure option makes sense here. If
it's only for code coverage, you can pass a -DCONFIG_VDI_BLOCK_SIZE to
the compiler. The main point is that I did not find any existing VDI
image in the wild which uses a non default block size, nor were there
any bug reports about that missing feature. As long as there are no
hints that variable block sizes not only exist in theory but also in
real live outside of QEMU, they should remain disabled in QEMU, too.


>>      } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
>>          error_setg(errp, "unsupported VDI image (block size %u is not %u)",
>>                     header.block_size, DEFAULT_CLUSTER_SIZE);
>> @@ -435,6 +440,18 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>>                     (uint64_t)header.blocks_in_image * header.block_size);
>>          ret = -ENOTSUP;
>>          goto fail;
>> +#if SIZE_MAX <= UINT32_MAX
>> +    } else if (header.blocks_in_image > SIZE_MAX / sizeof(uint32_t)) {
>> +        /* blocks_in_image * sizeof(uint32_t) must fit into size_t. */
>> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
>> +                   "only %zu are possible)",
>> +                   header.blocks_in_image, SIZE_MAX / sizeof(uint32_t));
>> +#endif
>> +    } else if (header.blocks_in_image > UINT64_MAX / header.block_size) {
>> +        /* blocks_in_image * block_size must fit into uint64_t. */
>> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
>> +                   "only %" PRIu64 " are possible)",
>> +                   header.blocks_in_image, UINT64_MAX / header.block_size);
> 
> Since header.blocks_in_image is a uint32_t, this means the driver may
> allocate up to 16GB of ram... that is my main concern here.
> 

People who want to run disk images with hundreds of terabyte should be
able to afford 16 GB of RAM :-) And if that memory is not available,
QEMU will tell them so (the memory allocators terminate the process in
this case).


> 
>>      } else if (!uuid_is_null(header.uuid_link)) {
>>          error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
>>          ret = -ENOTSUP;
>> @@ -691,6 +708,33 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
>>          options++;
>>      }
>>  
>> +#if defined(CONFIG_VDI_BLOCK_SIZE)
>> +    if (block_size == 0) {
>> +        return -EINVAL;
>> +    }
>> +#endif
>> +
> 
> Same concern as above with regards to dead code / new feature
> 

Activating that code would not be a good idea without prior tests and
existing real live examples. Otherwise we might create buggy images or
flood the world with incompatible VDI files.

Removing that optional code would make it harder if somebody needs it:
he or she would have to reinvent the wheel.

Yes, we have a dilemma here.

Regards
Stefan

Re: [Qemu-devel] [PATCH 2/2] vdi: add bounds checks for block related header fields (CVE-2014-0144)

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> Am 27.03.2014 20:49, schrieb Jeff Cody:
>> On Wed, Mar 26, 2014 at 10:38:05PM +0100, Stefan Weil wrote:
>>> (1) block_size must not be null.
>>>
>>> (2) blocks_in_image * 4 must fit into a size_t.
>>>
>>> (3) blocks_in_image * block_size must fit into a uint64_t.
>>>
>>> Header field disk_size already has a bounds check which now works
>>> because of modification (1) and (3).
>>>
>>> This patch was inspired by Jeff Cody's patch for the same problem.
>>>
>>> Cc: Jeff Cody <jcody@redhat.com>
>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>
>>> Hello Stefan, hello Jeff,
>>>
>>> I tried to improve your previous patch - maybe you want to improve it further
>>> or take parts from it to fix that CVE (of which I have no information).
>>>
>> 
>> Thanks!
>> 
>>> The patch was compiled on 32 and 64 bit Linux and cross compiled with MinGW-w64,
>>> but not tested otherwise.
>>>
>>> Regards
>>> Stefan
>>>
>>>  block/vdi.c |   50 +++++++++++++++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 45 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/vdi.c b/block/vdi.c
>>> index b832905..8fb59a1 100644
>>> --- a/block/vdi.c
>>> +++ b/block/vdi.c
>>> @@ -420,7 +420,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>>>                     header.sector_size, SECTOR_SIZE);
>>>          ret = -ENOTSUP;
>>>          goto fail;
>>> -#if !defined(CONFIG_VDI_BLOCK_SIZE)
>> 
>> Is this from qemu/master?  I don't see the above in the master
>> branch.
>
> It's from the previous patch (PATCH 1/2):
> http://patchwork.ozlabs.org/patch/334116/. You have to apply both
> patches in sequence.
>
>> 
>>> +#if defined(CONFIG_VDI_BLOCK_SIZE)
>>> +    } else if (header.block_size == 0) {
>>> +        error_setg(errp, "unsupported VDI image (block size is null)");
>>> +        ret = -EINVAL;
>>> +        goto fail;
>>> +#else
>> 
>> The above is essentially dead code, since CONFIG_VDI_BLOCK_SIZE is
>> commented out.  Should this be added to configure, so that it can be
>> compiled in like other options?  I think if it is worth putting the
>> code in the driver, it probably makes sense to have a configure option
>> for it.  Otherwise, in my opinion, it is generally best to excise dead
>> code.
>> 
>> With regards to adding this prior to QEMU 2.0, this would be more in
>> line with a feature addition, I think (i.e., the support of
>> non-default block sizes).
>> 
>
> Yes, it would be a very bad idea to activate variable block sizes for
> QEMU 2.0. I don't think adding a configure option makes sense here. If
> it's only for code coverage, you can pass a -DCONFIG_VDI_BLOCK_SIZE to
> the compiler. The main point is that I did not find any existing VDI
> image in the wild which uses a non default block size, nor were there
> any bug reports about that missing feature. As long as there are no
> hints that variable block sizes not only exist in theory but also in
> real live outside of QEMU, they should remain disabled in QEMU, too.
>
>
>>>      } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
>>>          error_setg(errp, "unsupported VDI image (block size %u is not %u)",
>>>                     header.block_size, DEFAULT_CLUSTER_SIZE);
>>> @@ -435,6 +440,18 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>>>                     (uint64_t)header.blocks_in_image * header.block_size);
>>>          ret = -ENOTSUP;
>>>          goto fail;
>>> +#if SIZE_MAX <= UINT32_MAX
>>> +    } else if (header.blocks_in_image > SIZE_MAX / sizeof(uint32_t)) {
>>> +        /* blocks_in_image * sizeof(uint32_t) must fit into size_t. */
>>> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
>>> +                   "only %zu are possible)",
>>> +                   header.blocks_in_image, SIZE_MAX / sizeof(uint32_t));
>>> +#endif
>>> +    } else if (header.blocks_in_image > UINT64_MAX / header.block_size) {
>>> +        /* blocks_in_image * block_size must fit into uint64_t. */
>>> +        error_setg(errp, "unsupported VDI image (number of blocks %u, "
>>> +                   "only %" PRIu64 " are possible)",
>>> +                   header.blocks_in_image, UINT64_MAX / header.block_size);
>> 
>> Since header.blocks_in_image is a uint32_t, this means the driver may
>> allocate up to 16GB of ram... that is my main concern here.
>> 
>
> People who want to run disk images with hundreds of terabyte should be
> able to afford 16 GB of RAM :-) And if that memory is not available,
> QEMU will tell them so (the memory allocators terminate the process in
> this case).

Hot plug the wrong image, kill your guest.  Not nice.  Suggest to use a
non-aborting memory allocation function for allocations whose size comes
from the user.  Precedence: we do that for guest RAM even though it's
not hot-plug, just to get a decent error message.

>> 
>>>      } else if (!uuid_is_null(header.uuid_link)) {
>>>          error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
>>>          ret = -ENOTSUP;
>>> @@ -691,6 +708,33 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
>>>          options++;
>>>      }
>>>  
>>> +#if defined(CONFIG_VDI_BLOCK_SIZE)
>>> +    if (block_size == 0) {
>>> +        return -EINVAL;
>>> +    }
>>> +#endif
>>> +
>> 
>> Same concern as above with regards to dead code / new feature
>> 
>
> Activating that code would not be a good idea without prior tests and
> existing real live examples. Otherwise we might create buggy images or
> flood the world with incompatible VDI files.
>
> Removing that optional code would make it harder if somebody needs it:
> he or she would have to reinvent the wheel.
>
> Yes, we have a dilemma here.

Code under #if 0 invariably rots.  If it's an untested sketch like here,
it starts life already rotten.  Trap for the unwary.

If you want to sketch some future feature in the code, please mark it
clearly, so nobody gets tempted to treat it as anything but a sketch.