Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1276443

Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

Show all headers | View raw

On Mon, Nov 23, 2015 at 8:46 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 23 Nov 2015 20:22:41 +0100
>
> The rbd_dev_destroy() function was called in two cases by the
> rbd_dev_probe_parent() function during error handling even if
> the passed variable contained a null pointer.
>
> * This implementation detail could be improved by adjustments
>   for jump targets according to the Linux coding style convention.
>
> * Drop an unnecessary initialisation for the variable "parent" then.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/block/rbd.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 24a757e..2ad9092 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -5148,7 +5148,7 @@ out_err:
>   */
>  static int rbd_dev_probe_parent(struct rbd_device *rbd_dev, int depth)
>  {
> -       struct rbd_device *parent = NULL;
> +       struct rbd_device *parent;
>         int ret;
>
>         if (!rbd_dev->parent_spec)
> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device *rbd_dev, int depth)
>         if (++depth > RBD_MAX_PARENT_CHAIN_LEN) {
>                 pr_info("parent chain is too long (%d)\n", depth);
>                 ret = -EINVAL;
> -               goto out_err;
> +               goto unparent_device;
>         }
>
>         parent = rbd_dev_create(rbd_dev->rbd_client, rbd_dev->parent_spec,
>                                 NULL);
>         if (!parent) {
>                 ret = -ENOMEM;
> -               goto out_err;
> +               goto unparent_device;
>         }
>
>         /*
> @@ -5176,15 +5176,15 @@ static int rbd_dev_probe_parent(struct rbd_device *rbd_dev, int depth)
>
>         ret = rbd_dev_image_probe(parent, depth);
>         if (ret < 0)
> -               goto out_err;
> +               goto destroy_device;
>
>         rbd_dev->parent = parent;
>         atomic_set(&rbd_dev->parent_ref, 1);
>         return 0;
> -
> -out_err:
> -       rbd_dev_unparent(rbd_dev);
> +destroy_device:
>         rbd_dev_destroy(parent);
> +unparent_device:
> +       rbd_dev_unparent(rbd_dev);
>         return ret;
>  }

Cleanup here is (and should be) done in reverse order.  We allocate
parent rbd_device and then link it with what we already have, so the
order in which we cleanup is unlink ("unparent"), destroy.

Changing it is just asking for use-after-free bugs.

Thanks,

                Ilya
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/2] block-rbd: Deletion of an unnecessary check SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-23 20:50 +0100
  [PATCH 1/2] block-rbd: Delete an unnecessary check before the  function call "rbd_dev_destroy" SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-23 20:50 +0100
    Re: [PATCH 1/2] block-rbd: Delete an unnecessary check before the  function call "rbd_dev_destroy" Ilya Dryomov <idryomov@gmail.com> - 2015-11-24 14:30 +0100
  [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-23 20:50 +0100
    Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection Ilya Dryomov <idryomov@gmail.com> - 2015-11-24 14:30 +0100
      Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-24 20:30 +0100
        Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection Ilya Dryomov <idryomov@gmail.com> - 2015-11-24 21:30 +0100
          Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-24 21:40 +0100
            Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection Ilya Dryomov <idryomov@gmail.com> - 2015-11-24 23:00 +0100
              Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-25 11:30 +0100
          Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-11-25 13:00 +0100
            Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection Ilya Dryomov <idryomov@gmail.com> - 2015-11-25 15:10 +0100
              Re: [PATCH 2/2] block-rbd: One function call less in  rbd_dev_probe_parent() after error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-11-25 16:30 +0100
          Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-26 09:00 +0100
            Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection Ilya Dryomov <idryomov@gmail.com> - 2015-11-26 12:50 +0100
              Re: block-rbd: One function call less in rbd_dev_probe_parent() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-11-26 14:50 +0100

csiph-web