Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-13891

ladvise: buffer allocated for ioctl is never released

Details

    • Bug
    • Resolution: Fixed
    • Minor
    • Lustre 2.14.0
    • Lustre 2.14.0, Lustre 2.12.6
    • 9223372036854775807

    Description

      There is a memory leak in the llapi_ladvise function. ladvise_hdr is never freed. 

      Attachments

        Activity

          [LU-13891] ladvise: buffer allocated for ioctl is never released
          pjones Peter Jones added a comment -

          Landed for 2.14

          pjones Peter Jones added a comment - Landed for 2.14

          Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/39610/
          Subject: LU-13891 utils: fix memory leak in llapi_ladvise()
          Project: fs/lustre-release
          Branch: master
          Current Patch Set:
          Commit: 50ce29a3b59a8c2984a928ea062e0b415739724d

          gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/39610/ Subject: LU-13891 utils: fix memory leak in llapi_ladvise() Project: fs/lustre-release Branch: master Current Patch Set: Commit: 50ce29a3b59a8c2984a928ea062e0b415739724d

          Jean-Yves Vet (contact@jean-yves.vet) uploaded a new patch: https://review.whamcloud.com/39610
          Subject: LU-13891 ladvise: Fix a memory leak
          Project: fs/lustre-release
          Branch: master
          Current Patch Set: 1
          Commit: 1cac9d5772fdaf5d9c10e30dfbe40ec14ac7e321

          gerrit Gerrit Updater added a comment - Jean-Yves Vet (contact@jean-yves.vet) uploaded a new patch: https://review.whamcloud.com/39610 Subject: LU-13891 ladvise: Fix a memory leak Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 1cac9d5772fdaf5d9c10e30dfbe40ec14ac7e321

          Hi Andreas, thanks for the review! Peter Jones kindly helped. I can now log in to gerrit. I've modified the patch and will push for review the new version as soon as I get push permissions set. 

          jyvet Jean-Yves Vet (Inactive) added a comment - Hi Andreas, thanks for the review! Peter Jones kindly helped. I can now log in to gerrit. I've modified the patch and will push for review the new version as soon as I get push permissions set. 

          Hi Jean-Yves, thanks for the patch. I'm just asking about an account for you.

           	rc = ioctl(fd, LL_IOC_LADVISE, ladvise_hdr);
           	if (rc < 0) {
          +		free(ladvise_hdr);
           		llapi_error(LLAPI_MSG_ERROR, -errno, "cannot give advice");
           		return -1;
           	}
          @@ -92,6 +93,7 @@ int llapi_ladvise(int fd, unsigned long long flags, int num_advise,
           					ladvise_iter->lla_lockahead_result;
           	}
           
          +	free(ladvise_hdr);
           	return 0;
           }
          

          It is preferred to have the error case use a goto and have only a single free() call at the end. Usig a single cleanup path avoids duplicate logic at each error site, and avoids similar leaks in the future.

          adilger Andreas Dilger added a comment - Hi Jean-Yves, thanks for the patch. I'm just asking about an account for you. rc = ioctl(fd, LL_IOC_LADVISE, ladvise_hdr); if (rc < 0) { + free(ladvise_hdr); llapi_error(LLAPI_MSG_ERROR, -errno, "cannot give advice" ); return -1; } @@ -92,6 +93,7 @@ int llapi_ladvise( int fd, unsigned long long flags, int num_advise, ladvise_iter->lla_lockahead_result; } + free(ladvise_hdr); return 0; } It is preferred to have the error case use a goto and have only a single free() call at the end. Usig a single cleanup path avoids duplicate logic at each error site, and avoids similar leaks in the future.

          Sorry, I don't have any account on review.whamcloud.com (same with id.whamcloud.com). I'll attach the patch to this ticket. 

          jyvet Jean-Yves Vet (Inactive) added a comment - Sorry, I don't have any account on review.whamcloud.com (same with id.whamcloud.com). I'll attach the patch to this ticket. 

          I'll submit a very small fix shortly. 

          jyvet Jean-Yves Vet (Inactive) added a comment - I'll submit a very small fix shortly. 

          People

            jyvet Jean-Yves Vet (Inactive)
            jyvet Jean-Yves Vet (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: