[bug #56806] Mem Leak in http_post_request

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #56806] Mem Leak in http_post_request

David GIRAULT-2
URL:
  <https://savannah.nongnu.org/bugs/?56806>

                 Summary: Mem Leak in http_post_request
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: schraubenkarl
            Submitted on: Mon 26 Aug 2019 08:03:59 AM UTC
                Category: apps
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: 2.1.1

    _______________________________________________________

Details:

After about 16 http POST requests pbuf_alloc returned NULL.

I figured out that in function http_post_request at line 1845

            pbuf_ref(q);
              return http_post_rxpbuf(hs, q);
            } else if (hs->post_content_len_left == 0) {
              q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);
              return http_post_rxpbuf(hs, q);
            } else {
              return ERR_OK;

the  memory allocated by  
   
    q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);

is never been deallocated.

After changing the code to:

    pbuf_ref(q);
              return http_post_rxpbuf(hs, q);
            } else if (hs->post_content_len_left == 0) {
              q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);
              err_t err;
              err = http_post_rxpbuf(hs, q);
              pbuf_free (q);
              return err;
            } else {
              return ERR_OK;

no memory leak is seen further.

Can somebody confirm the issue and also the solution?
thx




    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56806>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #56806] Mem Leak in http_post_request

David GIRAULT-2
Follow-up Comment #1, bug #56806 (project lwip):

http_post_rxpbuf sends the pbuf to http_post_get_response_uri


/**
 * @ingroup httpd
 * Called for each pbuf of data that has been received for a POST.
 * ATTENTION: The application is responsible for freeing the pbufs passed in!
 *
 * @param connection Unique connection identifier.
 * @param p Received data.
 * @return ERR_OK: Data accepted.
 *         another err_t: Data denied, http_post_get_response_uri will be
called.
 */
err_t httpd_post_receive_data(void *connection, struct pbuf *p);


So, your callback is responsible for doing a pbuf_unref.

Your fix is a workaround for your callback not doing its job, in one of the
code-paths. Take a look at the top two lines in your patch too:


              pbuf_ref(q);     // <------ this would be a problem too if your
httpd_post_receive_data doesn't do the pbuf_unref it is suppose to do
              return http_post_rxpbuf(hs, q);
            } else if (hs->post_content_len_left == 0) {
              q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);
              err_t err;
              err = http_post_rxpbuf(hs, q);
              pbuf_free (q);
              return err;
            } else {
              return ERR_OK;


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56806>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

Re: [bug #56806] Mem Leak in http_post_request

Sperlich, Klaus
Hi Stian,
thank's for your hint.
I changed the code, now it's working.
A comment in source code for proper pbuf_unref  in callback
could be helpfull.
Thx & Kind Regards


----------------------------------------
HARTING Electric GmbH & Co. KG  |  Postfach 14 73, 32328 Espelkamp  | Wilhelm-Harting-Straße 1, 32339 Espelkamp  |  www.HARTING.com
Pers. haftende Gesellschafterin: HARTING Electric Management GmbH
Geschäftsführer: Dipl.-Kfm. Edgar Peter Düning, Dipl.-Ing. Gunter Galtz, Dipl.-Ing. Norbert Gemmeke, Dr.-Ing. Andreas Imhoff
Sitz der Gesellschaft: Espelkamp  |  Amtsgericht Bad Oeynhausen  |  Register-Nr. HRA 5602  |  UST-Id Nr. DE813265676  |  WEEE-Reg.-Nr. DE 56507374


-----Ursprüngliche Nachricht-----
Von: Stian Sebastian Skjelstad <[hidden email]>
Gesendet: Montag, 26. August 2019 10:24
An: Sperlich, Klaus <[hidden email]>; Stian Sebastian Skjelstad <[hidden email]>; [hidden email]; [hidden email]
Betreff: [bug #56806] Mem Leak in http_post_request

Follow-up Comment #1, bug #56806 (project lwip):

http_post_rxpbuf sends the pbuf to http_post_get_response_uri


/**
 * @ingroup httpd
 * Called for each pbuf of data that has been received for a POST.
 * ATTENTION: The application is responsible for freeing the pbufs passed in!
 *
 * @param connection Unique connection identifier.
 * @param p Received data.
 * @return ERR_OK: Data accepted.
 *         another err_t: Data denied, http_post_get_response_uri will be
called.
 */
err_t httpd_post_receive_data(void *connection, struct pbuf *p);


So, your callback is responsible for doing a pbuf_unref.

Your fix is a workaround for your callback not doing its job, in one of the code-paths. Take a look at the top two lines in your patch too:


              pbuf_ref(q);     // <------ this would be a problem too if your
httpd_post_receive_data doesn't do the pbuf_unref it is suppose to do
              return http_post_rxpbuf(hs, q);
            } else if (hs->post_content_len_left == 0) {
              q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);
              err_t err;
              err = http_post_rxpbuf(hs, q);
              pbuf_free (q);
              return err;
            } else {
              return ERR_OK;


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56806>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/

_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

Re: [bug #56806] Mem Leak in http_post_request

Stian Skjelstad
From the httpd.h (says     ATTENTION: The application is responsible for freeing the pbufs passed in!  ) 

/**
* @ingroup httpd
* Called for each pbuf of data that has been received for a POST.
* ATTENTION: The application is responsible for freeing the pbufs passed in!
*
* @param connection Unique connection identifier.
* @param p Received data.
* @return ERR_OK: Data accepted.
*         another err_t: Data denied, http_post_get_response_uri will be called.
*/
err_t httpd_post_receive_data(void *connection, struct pbuf *p);  



But example in contrib does not call pbuf_unref, and should be patches, so a patch it still needed, but for the contrib

Stian Skjelstad


On Mon, Aug 26, 2019 at 10:38 AM Sperlich, Klaus <[hidden email]> wrote:
Hi Stian,
thank's for your hint.
I changed the code, now it's working.
A comment in source code for proper pbuf_unref  in callback
could be helpfull.
Thx & Kind Regards


----------------------------------------
HARTING Electric GmbH & Co. KG  |  Postfach 14 73, 32328 Espelkamp  | Wilhelm-Harting-Straße 1, 32339 Espelkamp  |  www.HARTING.com
Pers. haftende Gesellschafterin: HARTING Electric Management GmbH
Geschäftsführer: Dipl.-Kfm. Edgar Peter Düning, Dipl.-Ing. Gunter Galtz, Dipl.-Ing. Norbert Gemmeke, Dr.-Ing. Andreas Imhoff
Sitz der Gesellschaft: Espelkamp  |  Amtsgericht Bad Oeynhausen  |  Register-Nr. HRA 5602  |  UST-Id Nr. DE813265676  |  WEEE-Reg.-Nr. DE 56507374


-----Ursprüngliche Nachricht-----
Von: Stian Sebastian Skjelstad <[hidden email]>
Gesendet: Montag, 26. August 2019 10:24
An: Sperlich, Klaus <[hidden email]>; Stian Sebastian Skjelstad <[hidden email]>; [hidden email]; [hidden email]
Betreff: [bug #56806] Mem Leak in http_post_request

Follow-up Comment #1, bug #56806 (project lwip):

http_post_rxpbuf sends the pbuf to http_post_get_response_uri


/**
 * @ingroup httpd
 * Called for each pbuf of data that has been received for a POST.
 * ATTENTION: The application is responsible for freeing the pbufs passed in!
 *
 * @param connection Unique connection identifier.
 * @param p Received data.
 * @return ERR_OK: Data accepted.
 *         another err_t: Data denied, http_post_get_response_uri will be
called.
 */
err_t httpd_post_receive_data(void *connection, struct pbuf *p);


So, your callback is responsible for doing a pbuf_unref.

Your fix is a workaround for your callback not doing its job, in one of the code-paths. Take a look at the top two lines in your patch too:


              pbuf_ref(q);     // <------ this would be a problem too if your
httpd_post_receive_data doesn't do the pbuf_unref it is suppose to do
              return http_post_rxpbuf(hs, q);
            } else if (hs->post_content_len_left == 0) {
              q = pbuf_alloc(PBUF_RAW, 0, PBUF_REF);
              err_t err;
              err = http_post_rxpbuf(hs, q);
              pbuf_free (q);
              return err;
            } else {
              return ERR_OK;


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56806>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #56806] Mem Leak in http_post_request

David GIRAULT-2
In reply to this post by David GIRAULT-2
Update of bug #56806 (project lwip):

                  Status:                    None => Invalid                
             Open/Closed:                    Open => Closed                


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56806>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel